Update opentelementry-proto to 1.4 #6906
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. |
jhalliday
force-pushed
the
jh-profiling-f
branch
from
03b8c2d to
7f0700f
Compare
| 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.
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.
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.
Nit: Maybe take Collection instead of List for more flexibility here.
There was a problem hiding this comment.
List is already used elsewhere in methods serializing repeated fields. I agree that Collection is more flexible, but shouldn't obligate @jhalliday to make that switch here. If Jonathan wants to follow the "boy scout rule", and bundle the switch to Collection in with this PR, I'm open to that. Can also open a separate issue and resolve in a followup PR.
|
|
||
| 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.
Nit: Does it make sense to use java.util.Optional or does that just add headache here?
There was a problem hiding this comment.
We don't use Optional anywhere in this repository. The logic goes something like this:
- For external user facing APIs, optional creates a tri-state, with internals needing to check for null, present, and empty, since even with assuming non-null arguments and
@Nullableannotations where arguments are nullable, we often perform additional null checks. - For internal APIs, optinoal adds additional allocation vs.
@Nullable, and maintainers can / should accept higher cognitive load in exchange for better performance.
There was a problem hiding this comment.
Mostly changes to the experimental profiling protocol, with some additions to the serialization framework it uses.