Please consider a big refactor (including renaming probably) for this package before marking stable.

[atoulme]

trafficstars

          Please consider a big refactor (including renaming probably) for this package before marking stable.

Originally posted by @bogdandrutu in https://github.com/open-telemetry/opentelemetry-collector/issues/9805#issuecomment-2015423773

[bogdandrutu]

1. I think the "client" name of the package is too generic and confusing;
2. Some (if not all) of this can be part of the consumer package since it is "propagated" via context between consumers;
3. Better understand why we need to propagate any of these objects for the entire collector. Can some be "component" specific?

[TylerHelmuth]

If we move this to consumer we'd be adding a direct dependency between confighttp/configgrpc and consumer, but I think thats ok. Considering the functionality within client exists to help share context between components I think this makes sense.

When we do the move we can expand some of the naming to include the keyword Connection. So Info could become ConnectionInfo, Addr becomes ConnectionAddr, Auth becomes ConnectionAuth, and Metadata becomes ConnectionMetadata. It is kinda verbose, but directly conveys what the struct represents. NewContext can potentially become NewContextWithConnectionInfo.

I would prefer not to restrict the Info struct to any specific component type. We don't restrict how components make connections, so I think it is valid for any component to be allowed to add its connection to the context and pass it own down the component pipeline. It should also be valid for any component to ask if there is an Info struct in the current ctx.

[TylerHelmuth]

We took a shot at this via https://github.com/open-telemetry/opentelemetry-collector/pull/9996, but ultimately could not get consensus from approvers and maintainers on accepting the change. Ultimately, I think @jpkrohling summed up the feelings here:

I'm still not convinced we need this: while this change isn't backwards incompatible and there's a clear migration path, I don't think it's worth the hassle. Every authenticator out there (ours and external) will need to eventually migrate to the new package name. Even if the new name is really great, I don't think we should pursue this or similar changes before v1.

About the Metadata vs. AuthData, we could certainly have a separate discussion about unifying them, but I still see them as serving different purposes: metadata is typically just the headers from HTTP or gRPC, while AuthData doesn't necessarily has to be backed by a map of a slice of strings.

As someone else mentioned on yesterday's call: this package has been stable and serving it's purpose for a long time now, we shouldn't really be changing it in the name of stability for v1.

Thinking about 1.0 from an end-user perfective, we haven't been able to qualify what we'll gain from the suggested changes at this time. If, in the future, we need to come back to this issue, the 1.0 client module could be marked deprecated and the features could be added to 1.0 consumer as a feature add.