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

[receiver/jaegerreceiver] Add remote sampling deprecation warning #13408

Conversation

frzifus
Copy link
Member

@frzifus frzifus commented Aug 18, 2022

Signed-off-by: Benedikt Bongartz [email protected]

Description:

with #6632 and #6694 remote sampling is offered as extention. This pr adds a deprecation warning to the remote sampling endpoint provided in the receiver.

Link to tracking Issue: #6633 (part of)

Testing:

Documentation:

@frzifus frzifus requested a review from a team August 18, 2022 09:11
@frzifus frzifus requested a review from jpkrohling as a code owner August 18, 2022 09:11
func logSamplingDeprecation(logger *zap.Logger) {
once.Do(func() {
logger.Warn(
"Jaeger remote sampling support is deprecated and will be removed in future versions. Use the jaegerremotesampling extension instead.",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we point to a specific version here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume a feature flag is not needed? [documentation]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we point to a specific version here?

Yes, that makes sense.

I assume a feature flag is not needed?

I also don't see the need for a feature flag here.

@frzifus frzifus force-pushed the deprecate_jaeger_remote_sampling_on_receiver_#6633 branch from 760b72a to d16ce33 Compare August 18, 2022 09:17
@frzifus frzifus force-pushed the deprecate_jaeger_remote_sampling_on_receiver_#6633 branch 2 times, most recently from 2400acf to ac29939 Compare August 22, 2022 09:23
func logSamplingDeprecation(logger *zap.Logger) {
once.Do(func() {
logger.Warn(
"Jaeger remote sampling support is deprecated and will be removed in release v0.60.0. Use the jaegerremotesampling extension instead.",
Copy link
Member

@jpkrohling jpkrohling Aug 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps v0.61.0? We have 0.58.0 out already, this message would start appearing only at 0.59.0.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure 👍

@frzifus frzifus force-pushed the deprecate_jaeger_remote_sampling_on_receiver_#6633 branch from ac29939 to 828ee49 Compare August 22, 2022 15:06
@frzifus frzifus requested a review from jpkrohling August 22, 2022 15:08
@jpkrohling jpkrohling merged commit 2605cd7 into open-telemetry:main Aug 22, 2022
@frzifus frzifus deleted the deprecate_jaeger_remote_sampling_on_receiver_#6633 branch August 22, 2022 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants