-
Notifications
You must be signed in to change notification settings - Fork 840
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
Update opentelementry-proto to 1.4 #6906
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6906 +/- ##
============================================
- Coverage 90.23% 90.10% -0.14%
Complexity 6594 6594
============================================
Files 729 729
Lines 19800 19828 +28
Branches 1947 1950 +3
============================================
- Hits 17867 17866 -1
- Misses 1341 1369 +28
- Partials 592 593 +1 ☔ View full report in Codecov by Sentry. |
exporters/common/src/main/java/io/opentelemetry/exporter/internal/marshal/MarshalerUtil.java
Outdated
Show resolved
Hide resolved
exporters/common/src/main/java/io/opentelemetry/exporter/internal/marshal/MarshalerUtil.java
Show resolved
Hide resolved
...s/src/main/java/io/opentelemetry/exporter/otlp/internal/data/ImmutableAttributeUnitData.java
Outdated
Show resolved
Hide resolved
Looks like some merge conflicts need resolution. |
03b8c2d
to
7f0700f
Compare
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.
Just a few nits on things we can do to make our proto serialziation helper libs more clear.
public void serializeInt32(ProtoFieldInfo field, int value) throws IOException { | ||
if (value == 0) { | ||
return; | ||
} | ||
writeint32(field, value); | ||
} | ||
|
||
/** Serializes a protobuf {@code int32} field. */ |
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.
Nit: I'd explicitly call this out as serialization a {@code optional int32} field.
Yes, it's in the name of the method.
@@ -122,14 +122,27 @@ public void serializeSInt32(ProtoFieldInfo field, int value) throws IOException | |||
|
|||
protected abstract void writeSInt32(ProtoFieldInfo info, int value) throws IOException; | |||
|
|||
/** Serializes a protobuf {@code uint32} field. */ | |||
/** Serializes a protobuf {@code int32} field. */ |
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.
So - IIRC - this is actually how we serialize both uint32 and int32 in Java.
@@ -340,6 +353,25 @@ protected abstract void writeStartRepeatedVarint(ProtoFieldInfo field, int paylo | |||
|
|||
protected abstract void writeEndRepeatedVarint() throws IOException; | |||
|
|||
/** Serializes a {@code repeated int32} field. */ | |||
public void serializeRepeatedInt32(ProtoFieldInfo field, List<Integer> values) |
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.
Nit: Maybe take Collection
instead of List
for more flexibility here.
|
||
final class LocationMarshaler extends MarshalerWithSize { | ||
|
||
private static final LocationMarshaler[] EMPTY_REPEATED = new LocationMarshaler[0]; | ||
|
||
private final long mappingIndex; | ||
@Nullable private final Integer mappingIndex; |
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.
Nit: Does it make sense to use java.util.Optional
or does that just add headache here?
Mostly changes to the experimental profiling protocol, with some additions to the serialization framework it uses.