-
Notifications
You must be signed in to change notification settings - Fork 532
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
fix(detectors): reduce diag level on detectors failing to detect #2382
fix(detectors): reduce diag level on detectors failing to detect #2382
Conversation
@johnli-developer thanks for contributing to the project :) I see some other places in detectors where |
This PR is complete it's change to debug for detectors. Which one would you also like to change to debug? |
Here are some other ones:
That doesn't mean that this PR has to deal with all of them. But it won't "Fix: #2268" unless it gets all of them. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2382 +/- ##
==========================================
- Coverage 90.97% 90.52% -0.45%
==========================================
Files 146 157 +11
Lines 7492 7622 +130
Branches 1502 1571 +69
==========================================
+ Hits 6816 6900 +84
- Misses 676 722 +46
|
Sure, I will update them |
434e9bd
to
dd2a7cc
Compare
@trentm They are updated, could you help review the changes? |
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.
Looks good. Thanks.
From the original issue:
So my proposal would be:
reduce the level to debug
rephrase the messages
This handles the first point: reducing to diag.debug
.
I think the messages could benefit from some rephrasing to be less concerning, even for the user that has OTEL_LOG_LEVEL set to debug. However that could be done in a separate PR.
@johnli-developer I've set this up to merge when the PR is updated to the base branch. It looks like your fork repo doesn't allow me the access to do that. Can you update the base branch on this PR? I think the GitHub UI will show an "Update Branch" button for you. |
Head branch was pushed to by a user without write access
46ad53a
to
59fcdda
Compare
|
commit 1289068 Author: John Li <[email protected]> Date: Tue Aug 6 13:54:15 2024 +0800 fix: update detector log level Signed-off-by: John Li <[email protected]> Signed-off-by: John Li <[email protected]>
Signed-off-by: John Li <[email protected]>
59fcdda
to
2e32496
Compare
Which problem is this PR solving?
Short description of the changes