Skip to content

Add diagnostics for AKS #40988

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 16 commits into from
May 28, 2025
Merged

Add diagnostics for AKS #40988

merged 16 commits into from
May 28, 2025

Conversation

jeremydvoss
Copy link
Member

@jeremydvoss jeremydvoss commented May 8, 2025

Description

Diagnostics will now be enabled specifically for App Service attach and AKS attach. Also removes diagnostics for App Service manual instrumentation. However, those would not be triggered anyways.

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 - Distro Monitor OpenTelemetry Distro label May 8, 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 May 27, 2025 22:00
@Copilot Copilot AI review requested due to automatic review settings May 27, 2025 22:00
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 pull request updates diagnostic functionality to support both App Service and AKS attach scenarios while removing legacy diagnostics for manual instrumentation. Key changes include:

  • Replacing the constant _IS_DIAGNOSTICS_ENABLED with the new function _is_diagnostics_enabled() in multiple modules.
  • Updating unit tests across utilities and diagnostics modules to patch the new diagnostics function and support the AKS attach scenario.
  • Updating the CHANGELOG to reflect the new diagnostics enabled for AKS attach.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
sdk/monitor/azure-monitor-opentelemetry/tests/utils/test_utils.py Updated tests to validate diagnostics enablement using patch decorators for App Service, AKS, and Functions attach scenarios.
sdk/monitor/azure-monitor-opentelemetry/tests/diagnostics/test_status_logger.py Adjusted patch target from the old constant to the new _is_diagnostics_enabled() function.
sdk/monitor/azure-monitor-opentelemetry/tests/diagnostics/test_diagnostic_logging.py Same as above—updated patch to target _is_diagnostics_enabled().
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_utils/init.py Imported and defined the new _is_diagnostics_enabled() function to reflect diagnostic enablement logic.
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_diagnostics/status_logger.py Updated conditional check to call _is_diagnostics_enabled() instead of using the constant.
sdk/monitor/azure-monitor-opentelemetry/azure/monitor/opentelemetry/_diagnostics/diagnostic_logging.py Updated patch target from the constant to the _is_diagnostics_enabled() function.
sdk/monitor/azure-monitor-opentelemetry/CHANGELOG.md Documented new diagnostics behavior for AKS attach.
Comments suppressed due to low confidence (1)

sdk/monitor/azure-monitor-opentelemetry/tests/utils/test_utils.py:65

  • [nitpick] The order of patch decorator parameters may not match the typical decorator application order (which is bottom-to-top), potentially leading to confusion about which mock corresponds to which patched target. Consider reordering the mock parameters or adding inline comments to clarify the mapping.
def test_diagnostics_app_service_attach(self, attach_mock, app_service_mock, aks_mock, functions_mock):

@jeremydvoss jeremydvoss merged commit 8d2a46a into Azure:main May 28, 2025
19 checks passed
@jeremydvoss jeremydvoss mentioned this pull request May 29, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Monitor - Distro Monitor OpenTelemetry Distro
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants