Skip to content

New APM settings using telemetry. prefix deprecate ambiguous tracing.apm. settings.#104376

Merged
mosche merged 7 commits intoelastic:mainfrom
mosche:ES-7542_cleanup_apm_setting_names
Jan 30, 2024
Merged

New APM settings using telemetry. prefix deprecate ambiguous tracing.apm. settings.#104376
mosche merged 7 commits intoelastic:mainfrom
mosche:ES-7542_cleanup_apm_setting_names

Conversation

@mosche
Copy link
Copy Markdown
Contributor

@mosche mosche commented Jan 15, 2024

APM settings are renamed from tracing.apm.{name} to telemetry.tracing.{name} for tracing related settings. General APM settings are renamed to telemetry.{name}. The old settings are deprecated and applied as fallback.

(ES-7542)

…ng.apm." settings.

APM settings are renamed from "tracing.apm.{name}" to "telemetry.tracing.{name}" for tracing related settings. General APM settings are renamed to "telemetry.{name}". The old settings are deprecated and applied as fallback.

(ES-7542)
@mosche mosche added >non-issue :Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team labels Jan 15, 2024
@mosche mosche requested a review from a team January 15, 2024 15:45
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Copy link
Copy Markdown
Contributor

@pgomulka pgomulka left a comment

Choose a reason for hiding this comment

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

LGTM,
however we need to confirm the deprecation with BCC

),
OperatorDynamic,
NodeScope,
DeprecatedWarning
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

by marking a setting with DeprecatedWarning we are indicating that this will be removed. This is simialar to my comment on the affix settings. #104345 (comment)
as per @stu-elastic 's comment it would have to be consulted with Breaking changes committee

I think it is worth rising now. We don't necessarily need to plan to remove it in near future, but we want to suggest users not to use it (with just a warning - as you have used).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is there any docs on the deprecation process @pgomulka? I'm not sure what needs to be done next.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@pgomulka I've removed all deprecation warnings here. I'm going to move those into an independent PR to wrap up this part.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sounds good. feel free to merge

Copy link
Copy Markdown
Contributor

@stu-elastic stu-elastic left a comment

Choose a reason for hiding this comment

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

lgtm once randomness is removed

@mosche mosche merged commit 35cc9e1 into elastic:main Jan 30, 2024
@mosche mosche deleted the ES-7542_cleanup_apm_setting_names branch January 30, 2024 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Core Core issues without another label >non-issue Team:Core/Infra Meta label for core/infra team v8.13.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants