-
Notifications
You must be signed in to change notification settings - Fork 250
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
WIP: update otel deps to v0.99.0 #711
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.
Thank you! Looks good to me so far. We need to add a note in changelog.md though. I could write such a note for you, if you'd like? If you have already started writing a changelog note, or if you'd like to do it yourself, please feel free to push it yourself ;)
cd71599
to
622349a
Compare
sure that'd be great if you want to push a changelog entry to this branch |
Signed-off-by: Robbie Lankford <robert.lankford@grafana.com>
8579b5f
to
4fef741
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.
LGTM, few clarifying questions before merging though, and some action items post-merge.
I didn't check whether we missed any new settings introduced since the last upgrade; I put more attention on the code changes because it's easy to bring over new settings that we might have missed.
//TODO: Uncomment this test later. For now it's commented out because it depends on this package: | ||
// "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/prometheusreceiver" | ||
// ... which causes compilation issues with the Agent due to the fact that in Prometheus | ||
// textparse.MetricType changed to model.MetricType: | ||
// https://github.com/prometheus/prometheus/blob/12e317786b7ac864117f4be1a88a1aa29e5dcf9e/scrape/target.go#L89 | ||
// | ||
// See: | ||
// https://github.com/prometheus/prometheus/commit/8065bef172e8d88e22399504b175a8c9115e9da3 | ||
// https://github.com/prometheus/prometheus/commit/c83e1fc5748be3bd35bf0a31eb53690b412846a4 |
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.
Can we create an issue to track this?
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.
Doc changes are fine
Signed-off-by: Robbie Lankford <robert.lankford@grafana.com>
PR Description
Upgrade OpenTelemetry Collector dependencies to v0.99.0.
Which issue(s) this PR fixes
#631
Notes to the Reviewer
TODO:
PR Checklist