#### Description
Fix `protoID`s that were changed with breaking changes of the Profiling
protocol and were missed in the initial change #13758.
---------
Signed-off-by: Florian Lehner <florian.lehner@elastic.co>
#### Description
Protobuf has two possible encodings for a repeated field with wire type
VARINT/I32/I64:
- repeat the field tag for each value (like for repeated fields with
other wire types)
- use a "packed" representation: use a single field tag with wire type
LEN containing all the values
The packed representation is the default, but [the Protobuf
documentation](https://protobuf.dev/programming-guides/encoding/#repeated)
notes that:
> Protocol buffer parsers must be able to parse repeated fields that
were compiled as packed as if they were not packed, and vice versa. This
permits adding [packed=true] to existing fields in a forward- and
backward-compatible way.
The current Protobuf parser generated by pdatagen expects the packed
representation and returns an error if the "unpacked" representation is
provided instead, causing slightly off-spec clients to fail exporting
telemetry to the Collector. This PR fixes that by implementing parsing
of unpacked repeated fields.
#### Link to tracking issue
Fixes#13727
#### Testing
I added a test in `pdata/internal` which "manually" encodes a
`HistogramDataPoint` and an `ExponentialHistogramDataPoint_Buckets`
using unpacked fields, and checks that we successfully unmarshal them.
Profile payloads are also heavily affected, but I didn't add tests for
them to avoid the tests failing when there are breaking changes in the
profile proto.
This change should not affect other gRPC calls since it fallbacks to the
default grpc/proto encoding if requests are not pdata/otlp requests.
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Accessing the oneOf object (e.g. `Metric_Sum`) to reset the
`Metric_Sum.Sum` after `Metric_Sum` was added back to the free pool is a
race condition because `Metric_Sum` may already be re-used and then we
delete something that was just added.
Updates
https://github.com/open-telemetry/opentelemetry-collector/issues/13631
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
This feature is protected by the beta featuregate
`pdata.enableRefCounting` so that if unexpected problems occur to have a
simple way to disable the problematic logic. If nothing goes wrong for 2
versions this featuregate will be marked as stable.
Updates
https://github.com/open-telemetry/opentelemetry-collector/issues/13631
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
This feature is protected by a featuregate that is not yet enabled and
only marked as alpha. Preliminary results show > 20% CPU performance
improvement and at least 40% allocations improvement. I still need to
generate UnmarshalProto for AnyValue which should improve even more
results.
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Also, this PR improves JSON unmarshal logic by avoiding stack calls
caused by using the Read[Array|Object]CB and replace with simple
non-recursive iterations.
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
The only non-test change is in the Unmarshal proto but that generates
exactly the same code using the "defaultValue" for the message.
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Just in case there are problems, let's give users a way to disable the
new custom marshaling logic.
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
This removes lots of hand written JSON encoding code, and will help when
start using generated proto size/marshal/unmarshal.
This also adds tests for all previously hand written funcs.
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Add support to generate the proto Size by pdatagen. Not yet used, will
update after more tests are added.
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Remove the complication about internal/public for all funcs that only
need origin. Also improves significant testing.
This simplified a lot the fields (no more isCommon/isBaseCommon/etc.)
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
This PR allows to add tests for CopyOrig directly in the internal
package, also enables us to move more things to internal (including
tests) for Marshal/Unmarshal.
After this PR is merged we can generate the ProtoSize and other proto
helpers in internal.
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Main idea is to have all funcs that work on "origin" in internal, so
that we simplify access to them (see field access to this). Also will
remove duplicate generated code like copying the slice of KeyValue
(followup PR).
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Part of the effort in
https://github.com/open-telemetry/opentelemetry-collector/pull/13531.
Would also help with:
1. Help with trying to remove the need of manually define these extra
structs and generate them from the protoc parsed code.
1. Simplifies what we need to manually generate, only the proto type is
enough.
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
#### Description
Check if code and import statements are formatted, with additional
rules.
* https://golangci-lint.run/usage/formatters/#gofumpt
* https://go-critic.com/overview.html#paramtypecombine
This is the application of golangci-lint fmt ./... , the Makefile has
been update to include this behavior
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
This is a preparation to auto-generate the UnmarshalJSON using pdatagen.
By doing this we simplify to have all funcs on the Iterator and not
having some on the json package some on the Iterator.
Also, this is less error prune since it forces that all numbers are
correctly handle the string option.
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
This PR is part of an effort to remove usage of gogo proto in pdata and
switch to a model where we enhance the pdatagen tool to generate compact
(reflection free to avoid DCE disabling) and faster code.
The list of the PRs will do:
1. Auto generate the JSON marshaling logic (this PR).
1. Auto generate the JSON unmarshaling logic and replace manually
written code.
1. Auto generate the proto marshaling logic.
1. Auto generate the proto unmarshaling logic.
1. Remove the usage of auto-generate gogo proto code completely and
generate field members in the pdata objects.
1. Add support for lazy unmarshaling.
Performance improvements are more than 30x:
**Before:**
```
BenchmarkLogsMarshalJSON
BenchmarkLogsMarshalJSON-12 94 12541041 ns/op 8458944 B/op 196458 allocs/op
BenchmarkMetricsMarshalJSON
BenchmarkMetricsMarshalJSON-12 3 452309069 ns/op 350229024 B/op 8131400 allocs/op
BenchmarkProfilesMarshalJSON
BenchmarkProfilesMarshalJSON-12 30 36731071 ns/op 20317088 B/op 531652 allocs/op
BenchmarkTracesMarshalJSON
BenchmarkTracesMarshalJSON-12 8 132521260 ns/op 95920588 B/op 2208765 allocs/op
```
**After:**
```
BenchmarkLogsMarshalJSON
BenchmarkLogsMarshalJSON-12 3000 403587 ns/op 282465 B/op 1487 allocs/op
BenchmarkMetricsMarshalJSON
BenchmarkMetricsMarshalJSON-12 72 16772373 ns/op 11856529 B/op 72833 allocs/op
BenchmarkProfilesMarshalJSON
BenchmarkProfilesMarshalJSON-12 1460 849419 ns/op 674545 B/op 1145 allocs/op
BenchmarkTracesMarshalJSON
BenchmarkTracesMarshalJSON-12 271 4344647 ns/op 3010259 B/op 16237 allocs/op
```
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
When Entity was added, we forgot to update the unmarshal logic. Add e2e
tests so that this cannot happened in the future.
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Florian Lehner
Damien Mathieu
Jade Guiton
Bogdan Drutu
Matthieu MOREL