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

etcdserver: Replace value contents with value_size in request took too long warning #9822

Merged
merged 1 commit into from
Jun 8, 2018

Conversation

jpbetz
Copy link
Contributor

@jpbetz jpbetz commented Jun 7, 2018

This removes the logging of values for "request took too long warnings" that was introduced in etcd 3.2. We shouldn't log values. Value fields are likely contain sensitive data, make the logs difficult to scan, and have the potential to "flood" the logs exhausting disk space and/or starving the system disk IO.

This replaces all occurrences of value fields with a value_size field. Fortunately there are only two occurrences: (1) put requests and (2) txns that do a value compare.

Example output:

txn:

BEFORE:
2018-06-07 16:54:52.146394 W | etcdserver: request "header:<ID:7587830746628055043 > txn:<compare:<target:VALUE key:\"a\" value:\"1\" > success:<request_put:<key:\"b\" value:\"foo\" > > failure:<request_put:<key:\"c\" value:\"goo\\\"\" > > > " took too long (106.821µs) to execute

AFTER:
2018-06-07 16:37:42.945095 W | etcdserver: request "header:<ID:7587830746365689091 > txn:<compare:<target:VALUE key:\"a\" value_size:1 > success:<request_put:<key:\"b\" value_size:3 >> failure:<request_put:<key:\"c\" value_size:4 >>>" took too long (71.196µs) to execute

put:

BEFORE:
2018-06-07 16:54:58.694712 W | etcdserver: request "header:<ID:7587830746628055044 > put:<key:\"a\" value:\"this is a test\" > " took too long (106.008µs) to execute

AFTER:
2018-06-07 16:37:58.844290 W | etcdserver: request "header:<ID:7587830746365689092 > put:<key:\"a\" value_size:14 >" took too long (59.798µs) to execute

In order to preserve proto encoding of []byte fields, this introduces some private structs mirroring the request proto structs, but with value replaced with value_size.

cc @gyuho @wenjiaswe @mml

Copy link
Contributor

@gyuho gyuho left a comment

Choose a reason for hiding this comment

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

lgtm thanks!

@gyuho gyuho merged commit e22ab78 into etcd-io:master Jun 8, 2018
@wenjiaswe
Copy link
Contributor

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants