Create common package for reusing elastic specific attributes

[gregkalapos]

The PR adds a new module called common elasticattributes. This can be versioned separately from the outer opentelemetry-lib module.

Moved attributes.go to this new module. These attributes will be then shared across 1) the enrichment lib in this repo and 2) and the elastic intake receiver.

Update:
Discussed this in the Intake Service Otel sync meeting.

• To avoid super generic naming which isn't useful, instead of common, renamed the package to elasticattributes elasticattr, see: Create common package for reusing elastic specific attributes  #141 (comment)
• Dropped the Attribute[...] from all the attribute names - instead we can now write e.g. elasticattr.AgentName. A note on this: there seemed to be an agreement in the meeting that dropping the Attribute prefix from the attribute names makes sense, I just noticed OTel actually uses that prefix, e.g. that package has semconv.AttributeTelemetrySDKName. So by this move, we diverge from the OTel package. No strong opinion from me, I think dropping the Attribute prefix is fine.
• In this repo, we'll just use the package by replace github.com/elastic/opentelemetry-lib/elasticattributes => ./elasticattributes - my understanding is that for the intake receiver in the other repo, we'll still need to publish the module, but that can be done after this is ready and merged. update, see: Create common package for reusing elastic specific attributes  #141 (comment)

[kruskall]

issue: this is significantly increasing maintenance burden, could you expand on why we need a separate submodule instead of just using a package ?

[gregkalapos]

My thinking was separation of concerns.

I'd like to share the attributes across the intake receiver and the enricher lib from this repo (and module as it stands today - since this is a single module). If it's just a package, then in the intake receiver we'd pull in a module that contains (among other things) the enricher, which sounds very wrong to me. Making it a package would also definitely unblock my work, it'd just then look like a very strange design that I'd pull in a module into the receiver that has the enricher and the remappers which are all components that basically do the opposite of what the intake receiver does. (The intake receiver already has elastic specific data format and turns that into OTel format, the others we'd pull in have OTel data format and enrich/map that with/to elastic format).

What's the concern with increasing maintenance?

For completeness: Having said all that the opentelemetry-lib is already pulled in as an indirect reference via apm-data.

I think @lahsivjar has also some thinking about adding modules here as well.

[carsonip]

@kruskall is opposed to having submodule. @gregkalapos can you expand on why can't we just delete the go.mod in elasticattr dir? (I admit I missed some context here) In that case users of elasticattr can pull in the entire otel lib but just use the constants

[gregkalapos]

In that case users of elasticattr can pull in the entire otel lib but just use the constants

That's certainly possible, if we are ok with that, it's fine by me.

It's just that, the current module will still contain all the other things. E.g. in the intake receiver we'd also have the enricher as a dependency, which is almost the opposite functionality. Isn't that strange?

Other thing is releasing new version. Whatever package has a new version which we publish, all the other packages will also have the same new version - if let's say we update attributes once a year, but update remappers, enricher, and the new proposed config fetcher more frequently (let's say twice a month), then all the users of the attributes will also see a new version - potentially with a PR to bump the version.

What's more common in go? Where is the line between separating with modules vs. separating just packages?

Can please others elaborate on what's the concern here with module? I see increasing maintenance burden - but what does that contain? If that means, we'll need to publish separately, then that's true, but based on above I think it'd be better. But of course maybe there is something else I don't see here.

[kruskall]

Hey 👋

IMO we should use packages to encapsulate/separate code. It might make sense to use submodules when things are unrelated (see go agent modules for example) but I don't think this is the case, as you mentioned this would be a common module meant to be reused and all the other modules would use it. At that point it's no longer unrelated.

I'd like to share the attributes across the intake receiver and the enricher lib from this repo (and module as it stands today - since this is a single module). If it's just a package, then in the intake receiver we'd pull in a module that contains (among other things) the enricher, which sounds very wrong to me. Making it a package would also definitely unblock my work, it'd just then look like a very strange design that I'd pull in a module into the receiver that has the enricher and the remappers which are all components that basically do the opposite of what the intake receiver does. (The intake receiver already has elastic specific data format and turns that into OTel format, the others we'd pull in have OTel data format and enrich/map that with/to elastic format).

could you expand on this ? When you say pull do you mean the module zip ?
Go module pruning is designed in such a way that it won't pollute the module graph with indirect dependencies added in packages that you don't use. Moreover DCE should take care of not linking unused packages/code in the binary.

I see you made a similar point at #143 (comment) but if a downstream client does not use the agentcfg package they won't import/add the es client dependency.

What's the concern with increasing maintenance?

All the work needed to maintain a module will be multiplied. dependency updates, go version updates, releases/versioning, testing might be more difficult (need to test with all submodules).
The maintenance burden was significant even with the go agent (and they were unrelated module), I think we should only add a submodule when a package becomes a significant unrelated component.

For completeness: Having said all that the opentelemetry-lib is already pulled in as an indirect reference via apm-data.

This is really a bug :(
The component you linked above is only using the elasticapm processor. The processor is using some functions in the otlp package to translate data which causes the two to be too tighly coupled. The binary is probably not even linking the otlp consumer is DCE is working properly but the module graph is a mess.

It's just that, the current module will still contain all the other things. E.g. in the intake receiver we'd also have the enricher as a dependency, which is almost the opposite functionality. Isn't that strange?

I think this might boil down to the way we see packages and if they are enough to separate concerns. I would say no if the user is not importing/using the enricher because it won't deal with its dependency or link it in the binary. The only (in)visible effect will be some go files in the module zip.

[gregkalapos]

might make sense to use submodules when things are unrelated

Agreed. I think this is the key question here.

you mentioned this would be a common module meant to be reused and all the other modules would use it

No, not by all the other modules. The new module would be only used by 1 other in this module. There would be multiple things here that would not use the new module - because they are independent.

Specifically the repo has these packages:

• elastciattr (assuming this is a package): It's used by enrichment and by another module from the collector components repo
• agentcfg: fetches agent configs. Doesn't need any other package from this repo
• remappers: maps from Otel to elastic format. Doesn't use the enricher, the remappers, or the elasticattr
• enrichment : enriches traces from otel to elastic format. Independent from the remappers and the agent config fetcher, uses elasticattr

To me these seem very independent and if we count dependencies between those, then there is very little.

could you expand on this ? When you say pull do you mean the module zip ?

I mean there will be a require in the go.mod file which has these independent things.

All the work needed to maintain a module will be multiplied. dependency updates, go version updates, releases/versioning, testing might be more difficult (need to test with all submodules).

Ok, thanks for this info. For the releases and versioning I'd assume if components are independent, we should release and version independently.
On the go version update and the testing issues I don't have experience. It may be the blocker here.

I suggest, let's go with this: we have an intake sync in a few hours, I'll bring this up there.

I'll also remove the module from here and go with a package for now. We'll still have #144 and we can do a final decision there.

Does that sound ok?

Update: pushed a change to move on with the single module and adding this as a package. I'm still on the opinion these packages are independent, but that discussion can be continued on #144 (comment) and can be addressed separately.