Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[exporter/azuremonitor] Add metrics support #14916

Merged
merged 15 commits into from
Jan 25, 2023
Merged

[exporter/azuremonitor] Add metrics support #14916

merged 15 commits into from
Jan 25, 2023

Conversation

eedorenko
Copy link
Contributor

Description:
In addition to traces and logs azuremonitorexporter exports metrics to Azure Application Insights

Testing:
Added tests to cover metrics exporter code

Documentation:
Updated azuremonitorexporter readme to include metrics exporting functionality.

Fixes #14915

@eedorenko eedorenko requested review from a team and dashpole October 13, 2022 00:40
@eedorenko eedorenko requested review from avereshchak and removed request for dashpole October 13, 2022 18:53
avereshchak
avereshchak approved these changes Oct 14, 2022
View reviewed changes
Copy link

@avereshchak avereshchak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Oct 29, 2022
@slavomirs
Copy link

Hello guys, any update about this PR? We are implementing OTel with Azure backend and could really use the metrics functionality.

many thanks!

@github-actions github-actions bot removed the Stale label Nov 1, 2022
@eedorenko
Copy link
Contributor Author

@codeboten can someone review this PR?

@slavomirs slavomirs mentioned this pull request Nov 16, 2022
Closed
@pcwiese pcwiese mentioned this pull request Nov 16, 2022
Closed
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Nov 25, 2022
@stevehipwell
Copy link

Is one of the maintainers able to review this?

@github-actions github-actions bot removed the Stale label Nov 26, 2022
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Dec 12, 2022
@eedorenko eedorenko requested review from pcwiese, avereshchak and fatsheep9146 and removed request for avereshchak, pcwiese and fatsheep9146 December 21, 2022 22:22
@fatsheep9146
Copy link
Contributor

totally lgtm, some small nits @eedorenko

@fatsheep9146
Copy link
Contributor

@open-telemetry/collector-contrib-approvers
I think this pr is totally ready to merge, could anyone help review about this?

@fatsheep9146
Copy link
Contributor

@open-telemetry/collector-contrib-approvers @open-telemetry/collector-contrib-maintainer
I think this pr is totally ready to merge, could anyone help review about this?

@eedorenko
Copy link
Contributor Author

@codeboten @mx-psi @AnyBody
Can someone approve this PR?

@atoulme
Copy link
Contributor

atoulme commented Jan 25, 2023

Added to agenda of SIG meeting tomorrow for review https://docs.google.com/document/d/1r2JC5MB7GupCE7N32EwGEXs9V_YIsPgoFiLP4VWVMkE/edit#

1 similar comment
@atoulme
Copy link
Contributor

atoulme commented Jan 25, 2023

Added to agenda of SIG meeting tomorrow for review https://docs.google.com/document/d/1r2JC5MB7GupCE7N32EwGEXs9V_YIsPgoFiLP4VWVMkE/edit#

jpkrohling
jpkrohling approved these changes Jan 25, 2023
View reviewed changes
Copy link
Member

@jpkrohling jpkrohling left a comment •
edited
Loading

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.

@jpkrohling jpkrohling changed the title exporter/azuremonitor metrics [exporter/azuremonitor] Add metrics support Jan 25, 2023
@jpkrohling jpkrohling merged commit 9f774ef into open-telemetry:main Jan 25, 2023
@mx-psi
Copy link
Member

mx-psi commented Jan 25, 2023

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,

Error: metricexporter.go:44:22: cannot use exporter.config.InstrumentationKey (variable of type configopaque.String) as string value in assignment (typecheck)
					envelope.IKey = exporter.config.InstrumentationKey
					                ^

cc @jpkrohling @eedorenko could you have a look?

@jpkrohling
Copy link
Member

@eedorenko, please ping me on slack if you need a speedy review of the fix.

@eedorenko eedorenko mentioned this pull request Jan 25, 2023
Merged
@eedorenko
Copy link
Contributor Author

eedorenko commented Jan 25, 2023 •
edited
Loading

@mx-psi @jpkrohling Fixed in #18038

codeboten pushed a commit that referenced this pull request Jan 25, 2023
@eedorenko
[exporter/azuremonitor] Fix CI (#18038)
e77c342 
See #14916 (comment)
@mx-psi
Copy link
Member

mx-psi commented Jan 26, 2023

Thanks for the speedy fix @eedorenko :)

@atoulme atoulme mentioned this pull request Feb 24, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atoulme atoulme atoulme left review comments

@jpkrohling jpkrohling jpkrohling approved these changes

@avereshchak avereshchak avereshchak approved these changes

@fatsheep9146 fatsheep9146 fatsheep9146 approved these changes

@pcwiese pcwiese pcwiese approved these changes

Assignees

@codeboten codeboten

Labels
exporter/azuremonitor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[exporter/azuremonitor]
10 participants
@eedorenko @slavomirs @stevehipwell @fatsheep9146 @atoulme @mx-psi @jpkrohling @avereshchak @pcwiese @codeboten