Skip to content

Enable entra id via env var #40237

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

Merged
merged 12 commits into from
Apr 4, 2025
Merged

Enable entra id via env var #40237

merged 12 commits into from
Apr 4, 2025

Conversation

jeremydvoss
Copy link
Member

Description

Add entra id credential configuration via env var. Required for AKS attach.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@github-actions github-actions bot added the Monitor - Exporter Monitor OpenTelemetry Exporter label Mar 26, 2025
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

@jeremydvoss jeremydvoss marked this pull request as ready for review March 27, 2025 23:26
@Copilot Copilot AI review requested due to automatic review settings March 27, 2025 23:26
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for configuring Entra ID credentials via an environment variable, which is required for AKS attach. The changes include new tests validating env var–based authentication, an updated dependency on azure-identity in setup.py, and modifications to credential initialization in the exporter.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_base_exporter.py Added tests for environment variable credential behavior; note duplicate test case name issue.
sdk/monitor/azure-monitor-opentelemetry-exporter/setup.py Updated dependency to include azure-identity with a TODO for version clarification.
sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py Introduced _get_authentication_credential method and adjusted credential initialization; contains a debug print statement and a type annotation inconsistency.
sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/_constants.py Added constant for the authentication string environment variable.
sdk/monitor/azure-monitor-opentelemetry-exporter/README.md Updated documentation to include details on the new environment variable configuration.
sdk/monitor/azure-monitor-opentelemetry-exporter/CHANGELOG.md Documented the addition of Entra ID credential configuration via env var.
Comments suppressed due to low confidence (1)

sdk/monitor/azure-monitor-opentelemetry-exporter/tests/test_base_exporter.py:1107

  • Duplicate test method 'test_get_authentication_credential_no_aad' detected. Please rename one of the duplicate methods so both tests can run independently.
def test_get_authentication_credential_no_aad(self, mock_managed_identity):

@jeremydvoss jeremydvoss requested a review from lzchen April 4, 2025 21:11
@jeremydvoss jeremydvoss merged commit e72ab70 into Azure:main Apr 4, 2025
26 checks passed
@jeremydvoss jeremydvoss mentioned this pull request Apr 4, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Monitor - Exporter Monitor OpenTelemetry Exporter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants