[kafka] Add option to supply destination topic through context

[tylerbenson]

Description:

Add option to get destination topic from context. This allows for upstream connectors to control the destination without polluting the data being written.

Link to tracking Issue:
Fixes #34432

Testing:
Added unit tests.

Documentation:
Updated the component readme with the added setting.

[pavolloffay]

@codeboten can we move this PR forward?

[mx-psi]

Sorry to block this at the last minute but I don't think we should add an allowlist exception for this. Arbitrary Go API for building packages should be either in internal (if meant only for Collector contrib components) or in pkg (if meant also for external components, in which case I would like to have an example of which external component is going to use this)

Changes are addressed

[MovieStoreGuy]

I am not sure I want to encourage passing metadata via context, in my experience it has not lead to a good time and often caused more issues than what it attempts to solve.

I also think this could create non deterministic behaviour (from a user's perspective) where data is being routed to topics that don't exist or conditions of which the data is routed to a topic do not make sense.

I would prefer users using the routing connector and having several kafka exporters that is very clearly configured and understood how data is being routed and delivered.

[MovieStoreGuy]

I am not blocking this PR because it appears there is some interest in having some form of dynamic routing configuration, but this is not the way I suggest it should done.

[brettbuddin]

@MovieStoreGuy I mostly agree with you. The situation is a bit tricky for us, because we write our own custom components for Gateway Collectors that run in a multi-tenanted fashion. Due to the breadth of tenant scope and broker/topic structure it's not reasonable for everything to be explicitly declarative via configuration.

In a perfect world we'd derive a topic name and print it into the telemetry attributes and leverage that value using existing component capabilities. However, we see the topic name as an internal detail and not something we want to expose to tenants via the database (post-ingestion). This is the best option we've come up with so far, because the Collector component architecture doesn't provide a mechanism for agent-local metadata propagation other than telemetry data or context.Context. That's a reasonable design decision to make for most situations, and if our use case is far enough out-of-bounds we can find other ways of dealing with it.

[mx-psi]

@MovieStoreGuy Could you take a look at the comments above?

[tylerbenson]

Following up here. @pavolloffay @mx-psi @codeboten anything else I need to do?

[MovieStoreGuy]

@MovieStoreGuy I mostly agree with you. The situation is a bit tricky for us, because we write our own custom components for Gateway Collectors that run in a multi-tenanted fashion. Due to the breadth of tenant scope and broker/topic structure it's not reasonable for everything to be explicitly declarative via configuration.

Yeah, I get where you're coming from. I also think this problem isn't strictly limited to Kafka, you still have the same problem with something like OTLP exporter as an example if you wanted to include some amount of auth/meta data.

In a perfect world we'd derive a topic name and print it into the telemetry attributes and leverage that value using existing component capabilities. However, we see the topic name as an internal detail and not something we want to expose to tenants via the database (post-ingestion). This is the best option we've come up with so far, because the Collector component architecture doesn't provide a mechanism for agent-local metadata propagation other than telemetry data or context.Context. That's a reasonable design decision to make for most situations, and if our use case is far enough out-of-bounds we can find other ways of dealing with it.

The approach does go against the recommendations of go design paradigms suggest where Context isn't recommended to be used for business logic, however, I can see why we'd need it.

I am happy to approve this, but once there is a notion of supporting tenancy, I'd like for this approach to be deprecated in favour of what is to come, does that sound reasonable to you?

[tylerbenson]

I am happy to approve this, but once there is a notion of supporting tenancy, I'd like for this approach to be deprecated in favour of what is to come, does that sound reasonable to you?

I'm fine with that. I took this path because there wasn't another option for passing data along that wasn't embedded in the exportable data.

[tylerbenson]

I merged from main and resolved the conflicts.
The failing test doesn't look related to my change here.