Skip to content

Adding message to specify that the snippet can also be used in auto-i… #1817

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

didiergbenou-ms
Copy link
Contributor

…nstrumentation scenarios

Pull request guidance

Thank you for submitting your contribution to our support content! Our team works closely with subject matter experts in CSS and PMs in the product group to review all content requests to ensure technical accuracy and the best customer experience. This process can sometimes take one or more days, so we greatly appreciate your patience.

We also need your help in order to process your request as soon as possible:

  • We won't act on your pull request (PR) until you type "#sign-off" in a new comment in your pull request (PR) to indicate that your changes are complete.

  • After you sign off in your PR, the article will be tech reviewed by the PM or SME if it has more than minor changes. Once the article is approved, it will undergo a final editing pass before being merged.

Copy link

@didiergbenou-ms : Thanks for your contribution! The author(s) and reviewer(s) have been notified to review your proposed change.

Copy link
Contributor

Learn Build status updates of commit 560e652:

⚠️ Validation status: warnings

File Status Preview URL Details
support/azure/azure-monitor/app-insights/telemetry/opentelemetry-troubleshooting-python.md ⚠️Warning Details

support/azure/azure-monitor/app-insights/telemetry/opentelemetry-troubleshooting-python.md

  • Line 38, Column 124: [Warning: hard-coded-locale - See documentation] Link 'https://learn.microsoft.com/en-us/azure/azure-monitor/app/codeless-app-service?tabs=python' contains locale code 'en-us'. For localizability, remove 'en-us' from links to most Microsoft sites.
  • Line 38, Column 124: [Suggestion: docs-link-absolute - See documentation] Absolute link 'https://learn.microsoft.com/en-us/azure/azure-monitor/app/codeless-app-service?tabs=python' will be broken in isolated environments. Replace with a relative link.

For more details, please refer to the build report.

Note: Your PR may contain errors or warnings or suggestions unrelated to the files you changed. This happens when external dependencies like GitHub alias, Microsoft alias, cross repo links are updated. Please use these instructions to resolve them.

For any questions, please:

@@ -34,6 +34,8 @@ logger.addHandler(file)
logger.addHandler(stream)
...
```
> [!NOTE]
> In auto-instrumentation scenarios, this code snippet can also be added to your code to allow seeing internal logs of the [python auto-instrumentation agent](https://learn.microsoft.com/en-us/azure/azure-monitor/app/codeless-app-service?tabs=python).
Copy link
Contributor

@jeremydvoss jeremydvoss Mar 31, 2025

Choose a reason for hiding this comment

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

A couple issues with this. Thanks for reaching out. Firstly, logging.basicConfig does nothing if the root logger has already been configured which will almost always be the case in autoinstrumentation. So, the format specified would likely not be applied. Second, the code here only adds the file handler to the logger under name, i.e. the current file. So, if the current file is at the /foo/bar/init.py path, it would only capture logs created under the foo.bar.* namespace and not the namespace of our sdk, azure.monitor.opentelemetry.* . This applies for our manual sdk, too, not just autoinstrumentation.

Logs regarding autoinstrumentation can be seen at /var/log/applicationinsights/. See docs. However, if we want something more, we could discuss further.

Copy link
Contributor

Choose a reason for hiding this comment

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

logging.basicConfig would likely not work as intended even for manual instrumentation with the distro. I think this code likely came from the pre-distro, exporter-only era. We may want to redesign it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants