-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Setting to disable x-opaque-id in logs throttling #78911
Setting to disable x-opaque-id in logs throttling #78911
Conversation
Introduces a setting cluster.deprecation_indexing.x_opaque_id_used.enabled to disable use of x-opaque-id in RateLimitingFilter. This will be used for deprecation logs indexing and will not affect logging to files (it uses different instance of RateLimitingFilter with this flag enabled by default) Changes the indices backing a deprecation log data stream to be hidden. Refactors DeprecationHttpIT to be more reliable relates elastic#76292 closes elastic#77936
Pinging @elastic/es-core-infra (Team:Core/Infra) |
I split this PR from the main PR that was meant to enable deprecation logs indexing by default. |
...ion/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/deprecation/DeprecationHttpIT.java
Show resolved
Hide resolved
This applies to both indexed deprecation logs and logs emitted to log files. | ||
You can disable the use of `x-opaque-id` in throttling by changing | ||
`cluster.deprecation_indexing.x_opaque_id_used.enabled` to false | ||
See link:./server/src/main/java/org/elasticsearch/common/logging/RateLimitingFilter.java[RateLimitingFilter] |
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.
I'm not sure we should be linked to the source code. It implies we can't or don't want to explain what's going on. cc @jrodewig for his input.
Also, can we link x-opaque-id
to something descriptive?
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.
I thought I will link to a javadoc for more low level internals documentation.
@@ -28,9 +28,20 @@ | |||
import static org.elasticsearch.common.logging.DeprecatedMessage.KEY_FIELD_NAME; | |||
import static org.elasticsearch.common.logging.DeprecatedMessage.X_OPAQUE_ID_FIELD_NAME; | |||
|
|||
/** | |||
* A log4j filter used for throttling deprecation logs. Should be used in pair with a log4j2 appender. |
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.
I don't think we need to explain log4j here. You could link to https://logging.apache.org/log4j/2.x/manual/filters.html if you want to provide a reference.
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.
will do. I thought to comment about log4j so that someone won't bother trying to reuse this in some other context
server/src/main/java/org/elasticsearch/common/logging/RateLimitingFilter.java
Show resolved
Hide resolved
@@ -2,6 +2,7 @@ | |||
"template": { | |||
"settings": { | |||
"index": { | |||
"hidden" : true, |
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.
I don't think this change is related?
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.
not necessarily related, not even covered in testing here (index backing data stream is implementation detail)
do you recon we should do this in a separate PR?
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.
I don't suppose it matters either way, we can leave this in.
final DeprecationIndexingComponent component = new DeprecationIndexingComponent(client, | ||
environment.settings(), | ||
rateLimitingFilterForIndexing, | ||
WRITE_DEPRECATION_LOGS_TO_INDEX.getDefault(environment.settings())// default is true, enable on start. |
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.
Comment is wrong.
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.
true, will fix. I will leave the code for passing of the default value though.
I want to have the PR that will switch the default to true as small as possible. (previous problems with tests failing due this value being enabled on startup in CI)
...ion/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/deprecation/DeprecationHttpIT.java
Outdated
Show resolved
Hide resolved
...ion/qa/rest/src/javaRestTest/java/org/elasticsearch/xpack/deprecation/DeprecationHttpIT.java
Show resolved
Hide resolved
…asticsearch/xpack/deprecation/DeprecationHttpIT.java Co-authored-by: Rory Hunter <[email protected]>
@elasticmachine update branch |
@elasticmachine run elasticsearch-ci/part-2 |
) Introduces a setting cluster.deprecation_indexing.x_opaque_id_used.enabled to disable use of x-opaque-id in RateLimitingFilter. This will be used for deprecation logs indexing and will not affect logging to files (it uses different instance of RateLimitingFilter with this flag enabled by default) Changes the indices backing a deprecation log data stream to be hidden. Refactors DeprecationHttpIT to be more reliable relates elastic#76292 closes elastic#77936
…78982) Introduces a setting cluster.deprecation_indexing.x_opaque_id_used.enabled to disable use of x-opaque-id in RateLimitingFilter. This will be used for deprecation logs indexing and will not affect logging to files (it uses different instance of RateLimitingFilter with this flag enabled by default) Changes the indices backing a deprecation log data stream to be hidden. Refactors DeprecationHttpIT to be more reliable relates #76292 closes #77936 backport #78911
Introduces a setting cluster.deprecation_indexing.x_opaque_id_used.enabled to disable use of
x-opaque-id in RateLimitingFilter. This will be used for deprecation
logs indexing and will not affect logging to files (it uses different
instance of RateLimitingFilter with this flag enabled by default)
Changes the indices backing a deprecation log data stream to be hidden.
Refactors DeprecationHttpIT to be more reliable
relates #76292
closes #77936
gradle check
?