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

Parse numeric labels with a value of zero if units are specified. #619

Merged
merged 1 commit into from
Apr 6, 2021

Conversation

xyzsam
Copy link
Contributor

@xyzsam xyzsam commented Apr 6, 2021

By default, a numeric label with a value of zero is interpreted as an
empty label which will be ignored during parsing. However, a zero value
can be a valid label value. Instead of ignoring it altogether, parse it
as a zero-valued numeric label if num_unit is set.

By default, a numeric label with a value of zero is interpreted as an
empty label which will be ignored during parsing. However, a zero value
can be a valid label value. Instead of ignoring it altogether, parse it
as a zero-valued numeric label if num_unit is set.
@xyzsam xyzsam changed the title Parse numeric labels with a zero-value if units are specified. Parse numeric labels with a value of zero if units are specified. Apr 6, 2021
@google-cla google-cla bot added the cla: yes label Apr 6, 2021
@nolanmar511 nolanmar511 merged commit 17a10ee into google:master Apr 6, 2021
@aalexand
Copy link
Collaborator

aalexand commented Apr 7, 2021

@xyzsam @nolanmar511 It looks like we didn't add a test here? Looking at the *_test.go file it looks like the test data was updated, but I don't see an actual check added. I could be missing something.

@aalexand
Copy link
Collaborator

aalexand commented Apr 7, 2021

@xyzsam @nolanmar511 It looks like we didn't add a test here? Looking at the *_test.go file it looks like the test data was updated, but I don't see an actual check added. I could be missing something.

Oh, I guess it checks the whole serialize <-> deserialize loop? I assume the test failed before the fix?

copybara-service bot pushed a commit to google/tcmalloc that referenced this pull request Oct 10, 2022
google/pprof#619 interprets a non-zero unit as a
present field, even if proto3 causes the field to not be populated on the wire.

PiperOrigin-RevId: 480082958
Change-Id: I68ab12997cbdb31274ccc8ce9316b3e2dbd104b6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants