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

Do not throttle deprecated field logs #70009

Merged

Conversation

pgomulka
Copy link
Contributor

@pgomulka pgomulka commented Mar 5, 2021

The commit #55115 removed the possibility to directly force deprecation
log to be emitted. This means that usage of deprecated fields was
throttled and only one deprecation was logged. The key was common for
all fields = "deprecated_field".

This commit appends a used deprecated field name to prevent that
throttled.

  • Have you signed the contributor license agreement?
  • Have you followed the contributor guidelines?
  • If submitting code, have you built your formula locally prior to submission with gradle check?
  • If submitting code, is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed.
  • If submitting code, have you checked that your submission is for an OS and architecture that we support?
  • If you are submitting this code for a class then read our policy for that.

The commit elastic#55115 removed the possibility to directly force deprecation
log to be emitted. This means that usage of deprecated fields was
throtthled and only one deprecation was logged. The key was common for
all fields = "deprecated_field".

This commit appends a used deprecated field name to prevent that
throthling.
@pgomulka pgomulka added :Core/Infra/Logging Log management and logging utilities v7.13.0 labels Mar 5, 2021
@pgomulka pgomulka self-assigned this Mar 5, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Mar 5, 2021
@elasticmachine
Copy link
Collaborator

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

@@ -111,7 +113,7 @@ public void testDeprecatedMessageWithoutXOpaqueId() throws IOException {
public void testCompatibleLog() throws Exception {
withThreadContext(threadContext -> {
threadContext.putHeader(Task.X_OPAQUE_ID, "someId");
final DeprecationLogger testLogger = DeprecationLogger.getLogger("test");
final DeprecationLogger testLogger = DeprecationLogger.getLogger("org.elasticsearch.test");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why has this changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the config we used in that test associated deprecation.test with deprecation logger. And it is fine for DeprecationLogger in general as there is logic to handle it I am not really sure where we use it though https://github.com/elastic/elasticsearch/blob/master/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java#L64
It was enough for this test to just use DeprecationLogger.getLogger("test").

However ParsedField (and all other usages) are using DeprecationLogger with the class name. So the logger name would be something like org.elasticsearch....
So I thought I will change the name of the deprecation logger in that test.
In fact it is what we have in production log4j2.properties as well.

@pgomulka pgomulka merged commit 2e8e058 into elastic:master Mar 5, 2021
pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request Mar 5, 2021
The commit elastic#55115 removed the possibility to directly force deprecation
log to be emitted. This means that usage of deprecated fields was
throttled and only one deprecation was logged. The key was common for
all fields = "deprecated_field".

This commit appends a used deprecated field name to prevent that
throttled.
pgomulka added a commit that referenced this pull request Mar 9, 2021
The commit #55115 removed the possibility to directly force deprecation
log to be emitted. This means that usage of deprecated fields was
throttled and only one deprecation was logged. The key was common for
all fields = "deprecated_field".

This commit appends a used deprecated field name to prevent that
throttled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/Logging Log management and logging utilities Team:Core/Infra Meta label for core/infra team v7.13.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants