[collectd 6] Add OTLP support to the Write HTTP plugin. #4188
Conversation
There was a problem hiding this comment.
ChangeLog: Write HTTP plugin: Support for the OpenTelemetry protocol (OTLP) has been added.
There's no caller for format_json_open_telemetry(), and the otlp functions in the write_http plugin are just stubs?
=> For now I'm just commenting on the error logging...
|
Hey @eero-t, thanks for your comments! I'll address them shortly. This PR is still a draft and work in progress. Current state:
The existing formats encode each metric family individually and append the encoded metric family to a buffer. We could do the same for the JSON output but we can't do that for the binary protobuf output. What I'd prefer to do is what the write_open_telemetry plugin is also doing: stage metric families and convert them all to the desired format inside the flush callback/function. This allows us to (eventually) group metric families by their resource attributes, reducing the size of the generated payload. |
octo
force-pushed
the
6/write_http_otlp_json
branch
2 times, most recently
from
4337dce
to
b4457d7
Compare
|
This is now ready for review. This PR and #4180 share commits that introduce the |
Yes, please do that. |
Done: #4196 |
octo
force-pushed
the
6/write_http_otlp_json
branch
from
b4457d7
to
3edf728
Compare
|
Btw. according to this: influxdata/telegraf#6205 OpenTelemetry input plugin would mean collectd supporting also Jaeger clients (applications providing cluster request tracing data). |
octo
force-pushed
the
6/write_http_otlp_json
branch
from
3edf728
to
5b653f2
Compare
|
OpenTelemetry knows three distinct types: metrics, traces, and logs. At the moment we only have support for metrics. I think collectd could potentially send its logs to OpenTelemetry, but I'm not sure if collectd would provide value as traces and/or logs broker. |
|
Hm. Traces appear to be hierarchical / structured "logs": https://opentelemetry.io/docs/concepts/signals/traces/ => Yeah, that could indeed be awkward to support. Better to advance in smaller steps and get initial support ready first, and a release of it out. (If given application already has network port open, I guess it could as well offer it to outside the node, not just to a node-local trace broker like collectd.) |
|
Hey @eero-t, I'd like to merge this. Is there anything else you'd like to see changed before that? |
|
Thanks @eero-t! |
This also fixes the unit test by embedding PACKAGE_VERSION.
This reduces the amount of duplicate code being generated. Co-authored-by: Florian Forster <octo@collectd.org>
Also fixed the unit test to pass again.
Also updates the manpage to reflect this change.
Previously all responses were concatenated together until the buffer was full, at which point no additional responses were accessible.
octo
force-pushed
the
6/write_http_otlp_json
branch
from
1964ce6
to
84cc559
Compare
The protocol is described here: https://opentelemetry.io/docs/specs/otlp/#otlphttp
ChangeLog: Write HTTP plugin: Support for the OpenTelemetry protocol (OTLP) has been added.