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

[receiver/azuremonitor] Add support for Managed Identity and DefaultAzureCredential authentication #33584

Merged
merged 11 commits into from
Jul 10, 2024
Merged

[receiver/azuremonitor] Add support for Managed Identity and DefaultAzureCredential authentication #33584

merged 11 commits into from
Jul 10, 2024

Conversation

sriniketh923
Copy link
Contributor

Description:
Enhances azuremonitor receiver authentication by using managed identity or environment variables. I have extended the config parameter auth to support managed_identity and default_credentials. Also fix the issue where the errors were being ignored.

Link to tracking Issue:

  1. Fixes [receiver/azuremonitor] Handle errors; don't ignore them #31268
  2. No tracing issues for extenting auth methods

Testing: Tested on my Azure VMs with managed identity

Documentation: Added to README.md

@sriniketh923 sriniketh923 requested a review from a team June 15, 2024 02:47
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Jun 15, 2024 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@github-actions github-actions bot requested a review from nslaughter June 15, 2024 02:48
@nslaughter
Copy link
Contributor

The main intention of the PR looks great and the implementation is consistent with the existing code. CI should be triggered now.

@nslaughter
Copy link
Contributor

@codeboten are you able to approve the pending workflows?

@sriniketh923
Copy link
Contributor Author

@codeboten @nslaughter can you please review again?

nslaughter
nslaughter approved these changes Jun 20, 2024
View reviewed changes
Copy link
Contributor

@nslaughter nslaughter left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution and responses. LGTM

receiver/azuremonitorreceiver/scraper.go Show resolved Hide resolved
@sriniketh923
Copy link
Contributor Author

@codeboten anything else you would like me to do before you can merge this?

@brendtg
Copy link

brendtg commented Jul 2, 2024

@codeboten , any update on this? Or a timeline we can expect for a merge?

@crobert-1 crobert-1 added the ready to merge Code review completed; ready to merge by maintainers label Jul 9, 2024
@codeboten codeboten merged commit 1b4f045 into open-telemetry:main Jul 10, 2024
161 checks passed
@github-actions github-actions bot added this to the next release milestone Jul 10, 2024
@martinjt martinjt mentioned this pull request Jul 10, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codeboten codeboten codeboten approved these changes

@nslaughter nslaughter nslaughter approved these changes

@crobert-1 crobert-1 crobert-1 approved these changes

Assignees

@bogdandrutu bogdandrutu

Labels
ready to merge Code review completed; ready to merge by maintainers receiver/azuremonitor
Projects
None yet
Milestone
v0.105.0
Development

Successfully merging this pull request may close these issues.

[receiver/azuremonitor] Handle errors; don't ignore them
6 participants
@sriniketh923 @nslaughter @brendtg @codeboten @crobert-1 @bogdandrutu