-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[exporter/azuremonitor] Add metrics support #14916
[exporter/azuremonitor] Add metrics support #14916
Conversation
…y-collector-contrib into eedorenko/azuremonitor-metrics
…y-collector-contrib into eedorenko/azuremonitor-metrics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Hello guys, any update about this PR? We are implementing OTel with Azure backend and could really use the metrics functionality. many thanks! |
@codeboten can someone review this PR? |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Is one of the maintainers able to review this? |
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
totally lgtm, some small nits @eedorenko |
@open-telemetry/collector-contrib-approvers |
@open-telemetry/collector-contrib-approvers @open-telemetry/collector-contrib-maintainer |
@codeboten @mx-psi @AnyBody |
Added to agenda of SIG meeting tomorrow for review https://docs.google.com/document/d/1r2JC5MB7GupCE7N32EwGEXs9V_YIsPgoFiLP4VWVMkE/edit# |
1 similar comment
Added to agenda of SIG meeting tomorrow for review https://docs.google.com/document/d/1r2JC5MB7GupCE7N32EwGEXs9V_YIsPgoFiLP4VWVMkE/edit# |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't judge the business but looks sane to me, and given that the code owner approved it, I'm approving this as well.
This broke CI on main because of a type mismatch on configuration; see https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/4008300100/jobs/6882371209,
cc @jpkrohling @eedorenko could you have a look? |
@eedorenko, please ping me on slack if you need a speedy review of the fix. |
@mx-psi @jpkrohling Fixed in #18038 |
Thanks for the speedy fix @eedorenko :) |
Description:
In addition to traces and logs
azuremonitorexporter
exports metrics to Azure Application InsightsTesting:
Added tests to cover metrics exporter code
Documentation:
Updated
azuremonitorexporter
readme to include metrics exporting functionality.Fixes #14915