Skip to content

Conversation

@pgomulka
Copy link
Contributor

@pgomulka pgomulka commented Jul 19, 2019

Fields in JSON logs should be an escaped JSON fields. It is a broken json value at the moment
"stats": "["group1", "group2"]", -> "stats": "[\"group1\", \"group2\"]",
This should later be refactored into a JSON array of strings (the same as types in 7.x)

At the moment fields in JSON logs are escaped by default. therefore
inside that field should be en escaped string.
@pgomulka pgomulka added :Core/Infra/Logging Log management and logging utilities v8.0.0 labels Jul 19, 2019
@pgomulka pgomulka self-assigned this Jul 19, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@pgomulka pgomulka added the WIP label Jul 19, 2019
@pgomulka pgomulka changed the title WIP Fix stats in slow logs to be a escaped JSON Fix stats in slow logs to be a escaped JSON Jul 22, 2019
@pgomulka pgomulka removed the WIP label Jul 22, 2019
Copy link
Contributor

@alpar-t alpar-t left a comment

Choose a reason for hiding this comment

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

LGTM

@pgomulka pgomulka merged commit d7c84e1 into elastic:master Jul 22, 2019
pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request Jul 22, 2019
Fields in JSON logs should be an escaped JSON fields. It is a broken json value at the moment
"stats": "["group1", "group2"]", -> "stats": "[\"group1\", \"group2\"]",
This should later be refactored into a JSON array of strings (the same as types in 7.x)
pgomulka added a commit that referenced this pull request Jul 22, 2019
Fields in JSON logs should be an escaped JSON fields. It is a broken json value at the moment
"stats": "["group1", "group2"]", -> "stats": "[\"group1\", \"group2\"]",
This should later be refactored into a JSON array of strings (the same as types in 7.x)
jimczi added a commit to jimczi/elasticsearch that referenced this pull request Oct 22, 2019
This commit fixes the usage of JsonStringEncoder#quoteAsUTF8 in the SearchSlowLog.
JsonStringEncoder#getInstance should always be called to get a thread local object
but this assumption was broken by elastic#44642. This means that any slow log can throw
an AIOOBE since it uses the same byte array concurrently.

Closes elastic#48358
jimczi added a commit that referenced this pull request Oct 23, 2019
This commit fixes the usage of JsonStringEncoder#quoteAsUTF8 in the SearchSlowLog.
JsonStringEncoder#getInstance should always be called to get a thread local object
but this assumption was broken by #44642. This means that any slow log can throw
an AIOOBE since it uses the same byte array concurrently.

Closes #48358
jimczi added a commit that referenced this pull request Oct 23, 2019
This commit fixes the usage of JsonStringEncoder#quoteAsUTF8 in the SearchSlowLog.
JsonStringEncoder#getInstance should always be called to get a thread local object
but this assumption was broken by #44642. This means that any slow log can throw
an AIOOBE since it uses the same byte array concurrently.

Closes #48358
jimczi added a commit that referenced this pull request Oct 23, 2019
This commit fixes the usage of JsonStringEncoder#quoteAsUTF8 in the SearchSlowLog.
JsonStringEncoder#getInstance should always be called to get a thread local object
but this assumption was broken by #44642. This means that any slow log can throw
an AIOOBE since it uses the same byte array concurrently.

Closes #48358
jimczi added a commit that referenced this pull request Oct 28, 2019
This commit fixes the usage of JsonStringEncoder#quoteAsUTF8 in the SearchSlowLog.
JsonStringEncoder#getInstance should always be called to get a thread local object
but this assumption was broken by #44642. This means that any slow log can throw
an AIOOBE since it uses the same byte array concurrently.

Closes #48358
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 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants