chore(rfc): Datadog traces (from trace-agent) early support#9634
chore(rfc): Datadog traces (from trace-agent) early support#9634
Conversation
Signed-off-by: prognant <pierre.rognant@datadoghq.com>
Signed-off-by: prognant <pierre.rognant@datadoghq.com>
|
✔️ Deploy Preview for vector-project canceled. 🔨 Explore the source changes: 636b05e 🔍 Inspect the deploy log: https://app.netlify.com/sites/vector-project/deploys/61b9f1c2b84e18000854b4ca |
Signed-off-by: prognant <pierre.rognant@datadoghq.com>
| * User will be able to ingest trace from the trace agent | ||
| * Vector config would then consist in: `datadog_trace` source -> some filtering/enrichment transform -> | ||
| `datadog_trace` sink | ||
| * Datadog trace agent can be configured to send traces to any arbitrary endpoint using the `apm_config.apm_dd_url` | ||
| [config key](https://github.com/DataDog/datadog-agent/blob/34a5589/pkg/config/apm.go#L61-L87) | ||
| * This change is a pure addition to Vector, there will be no impact on existing feature |
There was a problem hiding this comment.
Wouldn't we want to support the datadog_agent -> datadog_trace pipeline? And enabling trace forwarding would be similar to DataDog/datadog-agent#9450 in that the user would configure the agent like so:
vector.traces.enabled: true
vector.traces.url: http://vector.company.tld/
There was a problem hiding this comment.
Indeed, this makes sense, and the other way around, if a user want to split traces ingestion from logs/metrics ingestion a separate source could still be added to the topology. This however might push for additional options to enable/disable some datatype per datadog_agent source.
Signed-off-by: prognant <pierre.rognant@datadoghq.com>
Signed-off-by: prognant <pierre.rognant@datadoghq.com>
|
|
binarylogic
left a comment
binarylogic
left a comment
There was a problem hiding this comment.
I would like to understand the decision better to use log events before approving this, since the high-level UX is affected by that.
|
See #9634 (comment) for my updated opinion. Once that is reflected in here I will approve. |
| To keep vector-core as generic as possible, the first implementation will decode datadog traces as `LogEvent`, the | ||
| resulting event will be deeper than usual but this should not be a problem. To keep track that those `LogEvent`s are | ||
| traces, we could add a simple `Option<bool>` to the `EventMetadata` struct. However this may not be immediately | ||
| required. While being naive this does not predates future vector evolution towards event traits by not adding a new | ||
| concrete type into the `Event` enum. |
There was a problem hiding this comment.
I was chatting with @lukesteensen this morning about this and I think there may be some confusion here about the modeling. We were thinking that we would add a new enum type, Trace, to Event and just have it wrap LogEvent. That is:
enum Event {
// ....
Trace(LogEvent)
}This would allow us to treat traces as a first-class type in Vector, but reuse most of the LogEvent implementation, for now at least.
| To keep vector-core as generic as possible, the first implementation will decode datadog traces as `LogEvent`, the | ||
| resulting event will be deeper than usual but this should not be a problem. To keep track that those `LogEvent`s are | ||
| traces, we could add a simple `Option<bool>` to the `EventMetadata` struct. However this may not be immediately |
There was a problem hiding this comment.
I think previously we had discussed using a new enum variant on Event to keep things straight and ensure traces cannot simply be used in all the same way logs can without us explicitly enabling it. So the current definition of Event would change to something like:
pub enum Event {
Log(LogEvent),
Metric(Metric),
Trace(LogEvent),
}This has the same benefit of reusing internals, but the additional benefit of locking down what we can do with traces in these early days. If they're treated as logs from a user or even topology perspective, we could run into weird incompatibilities or difficulty changing uses if/when we decide to implement a more specific API for traces.
There was a problem hiding this comment.
Oops, guess I should have refreshed before posting this 😄 @jszwedko was too quick.
Co-authored-by: Spencer Gilbert <spencer.gilbert@datadoghq.com>
|
@jszwedko @lukesteensen I don't remember we spoke about this this solution, or I missed something - my fault. Anyway it is a very convenient solution to have a dedicated trace type while leveraging existing code. I amended the RFC to reflect that. |
prognant
changed the title
9 participants
closes #9572
Quick summary of this RFC:
rendered