Reporter: allow extending samples with extra attributes

[athre0z]

This PR does two things:

Replace addProfileAttributes and attrKeyValue[T] with AttrTableManager

I found the previous design with the generic on addProfileAttributes rather inelegant. Firstly, there's no covariance on Go generics, so it would take two addProfileAttributes calls and an append to add both string and int attributes:

opentelemetry-ebpf-profiler/reporter/otlp_reporter.go

Lines 669 to 675 in 47e8410

	sample.Attributes = append(addProfileAttributes(profile, []attrKeyValue[string]{
	{key: string(semconv.ContainerIDKey), value: traceKey.containerID},
	{key: string(semconv.ThreadNameKey), value: traceKey.comm},
	{key: string(semconv.ServiceNameKey), value: traceKey.apmServiceName},
	}, attributeMap), addProfileAttributes(profile, []attrKeyValue[int64]{
	{key: string(semconv.ProcessPIDKey), value: traceKey.pid},
	}, attributeMap)...)

It also required reflection in addProfileAttributes for no good reason. IMHO this isn't really a good use-case for generics. In AttrTableManager this is replaced with two separate methods that each add just one attribute.

Secondly, a proper type for this results in a much cleaner interface for the second change discussed below.

Introduce SampleAttrProducer interface

This allows agent implementations to add custom sample attributes without maintaining a fork of the reporter implementation. It can gather extra meta-data when the trace is initially added and then emit extra attributes when the sample is sent out.

[athre0z]

Not sure why this was in the struct: it seems that simply making this a local temporary variable in the loop below is easier. I reworked it accordingly, but perhaps I'm missing something on why this was done like this originally.

[athre0z]

Same here.

[rockdaboot]

Overall looks better than the generics approach 👍

[florianl]

*profiles.Profile is internal to the package reporter. How are users of the package supposed to create it?
How should the integration look like, for a different kind of reporter, like for the OTel collector? see #208

From the updated testcase (TestGetSampleAttributes), where this API is used, it looks more like this API builds a parallel *profiles.Profile on top of an existing one just with attributes but without traces.

[athre0z]

How is this private? It's a public type from the OTEL proto lib?

From the updated testcase (TestGetSampleAttributes), where this API is used, it looks more like this API builds a parallel *profiles.Profile on top of an existing one just with attributes but without traces.

No, it's just populating the attribute table portion. I'll apply the change that Tim suggested here to make this clearer.

[athre0z]

Ah, I looked through #208 now, and I think what you are referring to is that it will be private in the future, replaced by the pdata type? I hadn't followed up with the collector reporter impl -- still catching up after ~1 month mostly away from this repo here -- apologies! Fortunately the changes that Tim suggested (and that I have applied now) should allow the type to generalize to allow using it in the new reporter implementation as well.

[athre0z]

Alternatively we could not take that any argument, and rename the type to AttrTableBuilder. In that case I'd then add a new method GetAttrTable() that returns the final built table. That'd change the workflow for the caller to:

• Create empty instance
• Populate it with AddXXX calls
• Take the final table with GetAttrTable() and manually assign it to the attribute table field

Perhaps more obvious than mutating the slice field in place through a pointer? No strong preference here.

[rockdaboot]

I thought about this as well (named it Get() to not repeat AttrTable), but it comes with the downside that you need to call this function when done. And this can be forgotten, while the solution in this PR is safer from accidental failure.

My not very strong preference would be to avoid an extra Get function call.

[florianl]

Can the returned any replaced with a (generic) type?
Maybe generic type parameters could help here? My worry is, that any might introduce subtle bugs by not using this API correctly.

[athre0z]

Not elegantly. It'd require us to also propagate the type parameter to Config and OTLPReporter structs.