-
Notifications
You must be signed in to change notification settings - Fork 786
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
[Exporter.Prometheus] remove redundant if in WriteUnicodeNoEscape #6077
base: main
Are you sure you want to change the base?
[Exporter.Prometheus] remove redundant if in WriteUnicodeNoEscape #6077
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, the change is correct. Maybe it is worth to add some comment about boundaries?
src/OpenTelemetry.Exporter.Prometheus.HttpListener/Internal/PrometheusSerializer.cs
Outdated
Show resolved
Hide resolved
…ometheusSerializer.cs Co-authored-by: Piotr Kiełkowicz <pkiekowicz@splunk.com>
@@ -102,16 +102,12 @@ public static int WriteUnicodeNoEscape(byte[] buffer, int cursor, ushort ordinal | |||
buffer[cursor++] = unchecked((byte)(0b_1100_0000 | (ordinal >> 6))); | |||
buffer[cursor++] = unchecked((byte)(0b_1000_0000 | (ordinal & 0b_0011_1111))); | |||
} | |||
else if (ordinal <= 0xFFFF) | |||
else // all other <= 0xFFFF which is ushort.MaxValue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SimonCropp Seems to be a style issue with how you dropped the comment. Sorry the rules in this repo are annoying strict 😄
Fixes #
Design discussion issue #
Changes
0x07FF is 2047
0xFFFF is 65535
Since there is
ordinal <= 0x07FF
check before theordinal <= 0xFFFF
then theordinal <= 0xFFFF
confition is always trueMerge requirement checklist
CHANGELOG.md
files updated for non-trivial changes