-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[all] Promote component.UseLocalHostAsDefaultHost
feature gate to beta
#33658
Conversation
aae4d15
to
b491204
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.
Did some basic grepping of 0.0.0.0
, below are references that are in the components referenced in the CHANGELOG.
internal/aws/proxy
: A couple tests are using 0.0.0.0
in their port binding, I'm not familiar enough with the logic to know how impactful this is to update to localhost
.
receiver/awsxray
: The README has lots of references 0.0.0.0
being the default address, I assume these should be updated now since localhost
will be the default?
receiver/jaeger
: The README has lots of references to 0.0.0.0
being the default address as well that should probably be updated. There are also a lot of tests still using 0.0.0.0
.
receiver/loki
: 0.0.0.0
references in README. Also, in its factory.go
, there are two default endpoint variables referencing 0.0.0.0
. One of these variables is unused, the other is only used in a test.
defaultGRPCBindEndpoint = "0.0.0.0:3600"
defaultHTTPBindEndpoint = "0.0.0.0:3500"
receiver/sapm
: References in README (along with feature gate reference that needs updated since it's enabled by default), and a few different test files.
@crobert-1 I don't feel the tests are something we need to change (if anybody disagrees, let me know), but I will open a PR to update the documentation, thank you for checking this! |
Fair point, they're especially still relevant while the feature gate can be disabled. I believe they'll need to be updated once it's graduated to stable though, unless I'm mistaken. |
@crobert-1 Updated READMEs, can you double check? |
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, thanks for making all of the documentation updates as well!
Description:
Counterpart to open-telemetry/opentelemetry-collector/pull/10352.
Link to tracking Issue: open-telemetry/opentelemetry-collector/issues/8510