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

json: fixes incorrect handling of error tag #1415

Merged
merged 3 commits into from
Feb 13, 2024
Merged

Conversation

codefromthecrypt
Copy link
Member

@codefromthecrypt codefromthecrypt commented Feb 13, 2024

Before, we didn't document that an existing tag with the same name as the errorTag parameter of the JSON writer would be retained. This documents it. It also fixes a bug where that key is not literally "error", for example, adding a tag named "exception".

I backfilled tests and adjusted benchmarks to make sure the result isn't worse than before.

Before

Benchmark                                                          Mode     Cnt    Score   Error   Units
ZipkinV2JsonWriterBenchmarks.sizeInBytes_bigClientSpan:gc.alloc.rate.norm  sample      15   0.042 ± 0.007    B/op
ZipkinV2JsonWriterBenchmarks.sizeInBytes_bigClientSpan:p0.99               sample           0.334           us/op
ZipkinV2JsonWriterBenchmarks.write_bigClientSpan:gc.alloc.rate.norm        sample      15  24.105 ± 0.008    B/op
ZipkinV2JsonWriterBenchmarks.write_bigClientSpan:p0.99                     sample           0.791           us/op

After

Benchmark                                                                    Mode     Cnt    Score   Error   Units
ZipkinV2JsonWriterBenchmarks.sizeInBytes_bigClientSpan:gc.alloc.rate.norm  sample      15    0.041 ± 0.006    B/op
ZipkinV2JsonWriterBenchmarks.sizeInBytes_bigClientSpan:p0.99               sample            0.334           us/op
ZipkinV2JsonWriterBenchmarks.write_bigClientSpan:gc.alloc.rate.norm        sample      15   24.101 ± 0.017    B/op
ZipkinV2JsonWriterBenchmarks.write_bigClientSpan:p0.99                     sample            0.750           us/op

Adrian Cole added 2 commits February 13, 2024 13:30
Signed-off-by: Adrian Cole <[email protected]>
Signed-off-by: Adrian Cole <[email protected]>
@codefromthecrypt codefromthecrypt merged commit 7158ba2 into master Feb 13, 2024
3 checks passed
@codefromthecrypt codefromthecrypt deleted the error-tag branch February 13, 2024 10:38
@codefromthecrypt
Copy link
Member Author

merging to unblock

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.

1 participant