Investigate how, and remove pdata `OtlpProtoSize`

[cuonglm]

@bogdandrutu @odeke-em could you please add more description to this issue?

[odeke-em]

@cuonglm essentially there is a method on the model/pdata.{Metrics, Traces, Logs} to return the size in bytes of the protocol buffer representation. It is used solely to reports stats of bytes sent in batch, per

$ git grep '.OtlpProtoSize' | grep -v _test.go
CHANGELOG.md:- Rename pdata Size to OtlpProtoSize (#2726)
model/pdata/logs.go:// OtlpProtoSize returns the size in bytes of this Logs encoded as OTLP Collector
model/pdata/logs.go:func (ld Logs) OtlpProtoSize() int {
model/pdata/metrics.go:// OtlpProtoSize returns the size in bytes of this Metrics encoded as OTLP Collector
model/pdata/metrics.go:func (md Metrics) OtlpProtoSize() int {
model/pdata/traces.go:// OtlpProtoSize returns the size in bytes of this Traces encoded as OTLP Collector
model/pdata/traces.go:func (td Traces) OtlpProtoSize() int {
processor/batchprocessor/batch_processor.go:    return bt.traceData.OtlpProtoSize()
processor/batchprocessor/batch_processor.go:    return bm.metricData.OtlpProtoSize()
processor/batchprocessor/batch_processor.go:    return bl.logData.OtlpProtoSize()

Answer

Yes, we can and shall remove it because the results it returns can be directly computed from invoking the protobuf generated .Size() per ExportRequest. I'll send a PR for it when I get back into the office tomorrow (after meetings)

[bogdandrutu]

What I think we need to do is, add a "Sizer" interface similar to "Marshaler" that a marshaler can implement. The otlp-protobuf Marshaler will implement and that will be used in the Batch processor where Size is now used.

This is what I was thinking about :)

[odeke-em]

What I think we need to do is, add a "Sizer" interface similar to "Marshaler" that a marshaler can implement. The otlp-protobuf Marshaler will implement and that will be used in the Batch processor where Size is now used.

This is what I was thinking about :)

Thank you @bogdandrutu! I didn't know about that use case for marshalers, but given that the the generated protobuf already produces .Size() for each generated struct, it seems much simpler and more direct to:

• Delete the method OtlpProtoSize from each type
• Add a helper function ProtoSizeForKnownTypes into model/pdata that'll return the sizes

Here is a prototype PR https://github.com/open-telemetry/opentelemetry-collector/pull/3692 of the prior approach I had suggested.

Please let me know what you think. After that, let me also work on another PR to implement your thinking @bogdandrutu to add the Sizer interface and examine the marshaler.

[bogdandrutu]

I do understand that, but the whole idea of the Marshaler was to hide the fact that for the moment internally pdata uses proto. That's why Sizer makes more sense.

[odeke-em]

I do understand that, but the whole idea of the Marshaler was to hide the fact that for the moment internally pdata uses proto. That's why Sizer makes more sense.

Sounds good to me @bogdandrutu! I'll mail the change for the marshaler and it'll be good to go tonight or early tomorrow. Thank you @bogdandrutu.

[bogdandrutu]

I think it should be a separate Interface that Marshaler implementations can implement as well (aka optional thing). What do you think?

[alolita]

@odeke-em pl respond to Bogdan's questions when you have a chance. Ty!

[kirbyquerby]

@bogdandrutu I've created #3818 -- is this along the lines of what you were thinking?

[alolita]

@bogdandrutu can you please review and comment?