telemetry: tag output by RDK request-metadata pass ID, not trace ID#222
telemetry: tag output by RDK request-metadata pass ID, not trace ID#222Vijay Vuyyuru (vijayvuyyuru) wants to merge 1 commit into
Conversation
| /// request metadata keys with "viam-metadata-" on the wire, so a value set as | ||
| /// "sanding-pass-id" is received as "viam-metadata-sanding-pass-id". | ||
| /// | ||
| inline constexpr std::string_view k_pass_id_metadata_key = "viam-metadata-sanding-pass-id"; |
There was a problem hiding this comment.
Hey, sorry, I guess I should have inquired further when we chatted about this. I don't think it makes sense for the universal robots module to be aware of sanding specific metadata fields. I think if we want to do this, then this needs to become configurable, under something like a new config key that describes what metadata field to use for tracing. Then you could configure it for a specific arm to be viam-metadata-sanding-pass-id and that would be fine.
Similarly, I think pass is a sanding specific term.
Could we achieve all of this by keeping the existing telemetry_output_path_append_traceid with its current semantics, and then introducing a new traceid_metadata_key or similar? If not set, it will use the existing traceparent stuff, as the key (just make it the default) but if it is set, it just looks for that field in the metadata.
There was a problem hiding this comment.
Hey sorry, I didnt mean to tag you on this yet, should have left as a draft. I agree with what youre saying here.
There was a problem hiding this comment.
No problem, just lmk when you are ready for me to have a look
Addresses review: the module should not know about sanding-specific
metadata or "pass" vocabulary. Keep telemetry_output_path_append_traceid
and its {trace_id} semantics unchanged, and add an optional
traceid_metadata_key config attribute. When set, the trace ID is read
directly from that request-metadata field; when unset, it is parsed from
the traceparent header as before. An operator configures the key per arm
(e.g. "viam-metadata-sanding-pass-id"), keeping the module domain-agnostic.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
0fff96d to
d24ecb8
Compare
2 participants
The telemetry output path was tagged by the OpenTelemetry trace ID parsed out of the incoming
traceparentheader. Callers (sanding) have moved off trace IDs to RDK's request-metadata propagation, which arrives on the wire asviam-metadata-sanding-pass-id, so the trace-based path no longer yields a usable tag.Read the pass ID directly from that metadata header (no W3C parsing) and tag the telemetry path with it. Rename the config surface to
telemetry_output_path_append_passid/{pass_id}, keeping the oldtelemetry_output_path_append_traceid/{trace_id}as a deprecated alias so existing machine configs keep working.