Skip to content
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

Sync defaults in otelcol.receiver.opencensus #6489

Closed
rfratto opened this issue Feb 22, 2024 · 2 comments · Fixed by #6682
Closed

Sync defaults in otelcol.receiver.opencensus #6489

rfratto opened this issue Feb 22, 2024 · 2 comments · Fixed by #6682
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.

Comments

@rfratto
Copy link
Member

rfratto commented Feb 22, 2024

This file notes that our defaults for otelcol.receiver.opencensus have desynced with upstream (or in this case, I'm not sure it was ever correct).

# Our defaults have drifted from upstream, so we explicitly set our
# defaults below (balancer_name and queue_size).
endpoint: database:4317

Changing a listening port like this would be a breaking change, but given how 4317 is used by another receiver by default, I think it's justifiable in this case; we should change our default for otelcol.receiver.opencensus to align with the upstream opencensusreceiver component.

@hainenber
Copy link
Contributor

Consider that OpenCensus has merged into OpenTelemetry and is encouraging people to migrate to the latter in addition to the OpenTelemetry folks's effort as well

I wonder if we should continue with the component to conserve engineering powers 🤖 (my 2c ofc)

rfratto added a commit to rfratto/agent that referenced this issue Mar 14, 2024
This commit re-syncs `otelcol.receiver.opencensus` with the upstream
defaults, namely changing the default listen port from 4317 to 55678.

This is considered a breaking change; a section has been written in the
upgrade guide notifying users to explicitly set the endpoint to the
previous port to retain old behavior when upgrading.

Closes grafana#6489.
@rfratto
Copy link
Member Author

rfratto commented Mar 14, 2024

This one makes me worried because we're so close to releasing 1.0, so if we're going to make breaking changes it really needs to be now. Even with encouraging people to migrate, it can sometimes take organizations a really long time to move off of old tooling, so in this case I think it's better to make sure we're aligned with upstream going into 1.0.

I spent a few minutes to fix this and opened #6682 so we can move on :)

rfratto added a commit that referenced this issue Mar 14, 2024
This commit re-syncs `otelcol.receiver.opencensus` with the upstream
defaults, namely changing the default listen port from 4317 to 55678.

This is considered a breaking change; a section has been written in the
upgrade guide notifying users to explicitly set the endpoint to the
previous port to retain old behavior when upgrading.

Closes #6489.
@github-actions github-actions bot added the frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed. label Apr 14, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Locked due to a period of inactivity. Please open new issues or PRs if more discussion is needed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants