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

Avoid revalidating labelset keys on each metric observation #144

Closed
wants to merge 1 commit into from

Conversation

dmagliola
Copy link
Collaborator

When declaring a metric, we declare what label keys will be used for it.
At that point, we validate that those are all valid keys (symbols,
match a given regex, etc).

Then, on each observation of the metric, we validate that the keys passed
in for the labels match the ones we were originally expecting, to make
sure all of those labels were set, but no others.

We were also, at that point, validating that the keys passed in are valid.
This validation is pretty slow, and it's redundant, since keys that aren't
valid won't match the expected ones anyway, so we can just compare just
that those match. This has quite a pronounced effect on performance.

@dmagliola dmagliola force-pushed the dont_revalidate_symbols_on_increment branch from 0f6ec7e to d2dc049 Compare July 17, 2019 11:43
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 99.929% when pulling 0f6ec7e on dont_revalidate_symbols_on_increment into 607a43a on master.

@coveralls
Copy link

coveralls commented Jul 17, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 7431d45 on dont_revalidate_symbols_on_increment into 607a43a on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.07%) to 99.929% when pulling 0f6ec7e on dont_revalidate_symbols_on_increment into 607a43a on master.

When declaring a metric, we declare what label keys will be used for it.
At that point, we validate that those are all valid keys (symbols,
match a given regex, etc).

Then, on each observation of the metric, we validate that the keys passed
in for the labels match the ones we were originally expecting, to make
sure all of those labels were set, but no others.

We were also, at that point, validating that the keys passed in are valid.
This validation is pretty slow, and it's redundant, since keys that aren't
valid won't match the expected ones anyway, so we can just compare just
that those match. This has quite a pronounced effect on performance.
@dmagliola dmagliola force-pushed the dont_revalidate_symbols_on_increment branch from d2dc049 to 7431d45 Compare July 17, 2019 11:44
@dmagliola
Copy link
Collaborator Author

Turns out, this has a massive impact if we remove the cache (which is one of the options we were contemplating to solve one of the outsanding issues). Keeping the cache and doing this means we don't gain that much, so it doesn't make sense to have this isolated PR. Closing, and this code will show up in another PR, if that's the direction we go.

@dmagliola dmagliola closed this Jul 17, 2019
@dmagliola dmagliola deleted the dont_revalidate_symbols_on_increment branch July 17, 2019 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants