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

fix(core): ignore labels with empty value #579

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gjasny
Copy link
Collaborator

@gjasny gjasny commented Apr 6, 2022

The Data Model spec states:

A label with an empty label value is considered
equivalent to a label that does not exist.

@gjasny gjasny marked this pull request as draft April 6, 2022 21:00
The Data Model spec states:
> A label with an empty label value is considered
> equivalent to a label that does not exist.
@gjasny gjasny force-pushed the ignore-empty-labels branch from 651092f to 396cc23 Compare April 6, 2022 21:00
@gjasny gjasny changed the title fix(core): Ignore labels with empty value fix(core): ignore labels with empty value Apr 6, 2022
@BenjaminW3
Copy link
Contributor

BenjaminW3 commented Feb 24, 2023

Conceptually this looks good to me.

I am not sure about the performance impact. In our codebase we have some places where we dynamically do family.Add({{"key", value}}).Increment(); on each observation. This would now end up with an additional copy of the labels. Compared to the map lookup that this produces, the copy likely is not relevant.

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.

2 participants