-
Notifications
You must be signed in to change notification settings - Fork 155
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
Add basicauth extension to contrib distribution #62
Conversation
@@ -13,6 +13,7 @@ extensions: | |||
gomod: go.opentelemetry.io/collector v0.42.0 | |||
- gomod: github.com/open-telemetry/opentelemetry-collector-contrib/extension/asapauthextension v0.42.0 | |||
- gomod: github.com/open-telemetry/opentelemetry-collector-contrib/extension/awsproxy v0.42.0 | |||
- gomod: github.com/open-telemetry/opentelemetry-collector-contrib/extension/basicauthextension v0.42.0 |
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.
It doesn't exist at this version though, does it?
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.
Should I temporarily use commit ref like v0.0.0-20220119171801-207f2c3db0f4
or what is the recommended approach for specifying unreleased components?
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.
I think this would be the first case where a component is added to the same version. I think I would prefer to update all versions to something like what you listed (but with v0.42.0-... instead of v0.0.0-...). If you could do it in two commits, it would be great: the first commit updating all existing deps, the second adding the new component.
@codeboten, this should be included for 0.43.0.
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.
For pseudo-versions I can either use v0.0.0
or v0.42.1-0
. It fails for me if I try to specify v0.42.0
like v0.42.0-20220107162206-6a1c2471092b
for contrib deps.
I can rename versions to v0.42.1-0.20220119200947-29b515d5d9c0
format if you actually want me to bump them to the latest revision in main
.
For basicauth
I can only use v0.0.0
as it doesn't have preceding versions, this will change with v0.43.0
.
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.
The idea is to be able to replace them all at once. Otherwise, one could miss that one of the modules is at a different version, when they all should have the same by the time it's released. If the only one that works for everything is 0.0.0, then so be it :-)
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.
Gotcha.
3720148
to
adc0eea
Compare
I didn't manage to build any distribution which has contrib revision >= open-telemetry/opentelemetry-collector-contrib@fff0a08 which bumped prometheus deps by few major releases and was right before my change. Will wait for dependency hell to be resolved in contrib before proceeding. I see there is some action going on. :)
|
Managed to build both distributions by adding explicit replaces. I guess they can be removed as soon as there is 0.43.0 release. |
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, but for the future, we need to figure out a better way to do this.
cc @codeboten
@jpkrohling The main issue I see are the components which reference other components, e.g. https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/0ae61c5528951ce7ddac72200c6954377f0c1fdc/receiver/prometheusexecreceiver/go.mod#L7 and |
See open-telemetry/opentelemetry-collector-contrib#7167