-
Notifications
You must be signed in to change notification settings - Fork 3k
Log error for 401/403 to assist with customer troubleshooting #41767
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
Conversation
There was a problem hiding this 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 enhances error visibility by logging 401/403 responses for customer troubleshooting, streamlines retrieval of the Application Insights authentication string, and updates the changelog to document the new logging.
- Added specific error logs for 401 (Unauthorized) and 403 (Forbidden) in
_transmit
- Moved
os.getenv
call outside the conditional in_get_authentication_credential
to consolidate variable assignment - Updated CHANGELOG to reflect the new logging behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
azure/monitor/opentelemetry/exporter/export/_base.py | Log specific messages on 401/403, adjust environment variable usage for authentication string |
CHANGELOG.md | Document the addition of 401/403 logging for troubleshooting |
Comments suppressed due to low confidence (1)
sdk/monitor/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py:250
- Consider adding unit tests to cover the new logging branches for 401 and 403 status codes, ensuring these log messages are triggered as expected.
# Log error for 401: Unauthorized, 403: Forbidden to assist with customer troubleshooting
...or/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py
Show resolved
Hide resolved
...or/azure-monitor-opentelemetry-exporter/azure/monitor/opentelemetry/exporter/export/_base.py
Show resolved
Hide resolved
Are there any tests for this change? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.