-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 Collector configuration issues #3443
Fix Collector configuration issues #3443
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.
looks good overall, minor discussion/suggestion point around the endpoint URLs
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. @open-telemetry/collector-approvers please take a look!
5162477
to
fd81f8b
Compare
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.
Very nice update, thanks! ✨
See inline for suggested copyedits.
(This page uses blockquote syntax for lightweight side notes, which I think is fine for now.)
Co-authored-by: Patrice Chalin <[email protected]>
Co-authored-by: Patrice Chalin <[email protected]>
Co-authored-by: Patrice Chalin <[email protected]>
Co-authored-by: Patrice Chalin <[email protected]>
…heletterf/opentelemetry.io into theletterf-fix-config-collector-issues
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.
Another quick pass.
Co-authored-by: Patrice Chalin <[email protected]>
Co-authored-by: Patrice Chalin <[email protected]>
Co-authored-by: Patrice Chalin <[email protected]>
Co-authored-by: Patrice Chalin <[email protected]>
@svrnm This one can be merged, I think. |
We need approval from someone from @open-telemetry/collector-approvers |
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.
Thanks for the improvements! Just a couple of comments
…heletterf/opentelemetry.io into theletterf-fix-config-collector-issues
@codeboten Changes applied! |
@codeboten does this look good? |
@theletterf can you take a look at the conflict? I think this is related to some changes to the images I introduced yesterday, so it should not have any impact on the content. |
@svrnm Should be solved! |
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.
Thanks @theletterf!
Fixes #1697 and adds some rewrites for clarity.
Preview: https://deploy-preview-3443--opentelemetry.netlify.app/docs/collector/configuration/