Irrelevant tracing labels hardcoded into configuration

[aThorp96]

Hi folks.

I recently discovered PaC has explicit labels in its OTEL traces application, component, and action. These are somewhat confusing because they appear to be unrelated to any Pipelines as Code or Tekton model.

Background

The application, component, and action labels are pulled from the pipelinerun's labels delivery.tekton.dev/* which is not a meaningful or defined namespace in Tekton. These labels appear to be coming from konflux-ci's model, which builds on top of PaC. I don't think these labels belong in Pipelines as Code since they have no relation to TektonCD; they are downstream abstractions which were included based on the argument that the labels were "concepts that any platform can set on its PipelineRuns."

I'd like to remove the labels, however they clearly represent a valid need. Configuring custom trace attributes based on PipelineRun labels enables downstream consumers to better organize their tracing. And since PaC is the entry point a PipelineRun and creates the root OTEL trace, it is the ideal place to configure custom trace attributes.

Proposal

We can enable custom labels without hardcoding whatever downstream consumers want to bike-shed using simple namespacing in the configuration. Additionally, if the downstream consumers like Konflux have different domain models or need additional labels, a more flexible configuration enables them to add their own labels/attributes without hardcoding them upstream.

1. Remove references to delivery.tekton.dev/action, delivery.tekton.dev/application, and delivery.tekton.dev/component
2. Define a new configuration setting via regex, custom-tracing-label.([a-z_./-]+) (custom-tracing-label.<key>) where the key capture-group is the custom tracing attribute's key and the setting's value is the pipelineRun label where the value is pulled from. For example parity with the current behaviour would be to configure like so:

custom-tracing-label.cicd.pipeline.action.name: "delivery.tekton.dev/action"
custom-tracing-label.delivery.tekton.dev.application: "delivery.tekton.dev/application"
custom-tracing-label.delivery.tekton.dev.component: "delivery.tekton.dev/component"

3. (optional) Define a new configuration setting custom-tracing-label-domain which would simplify the configuration. E.g. the below would map the PLR label delivery.tekton.dev/action to the OTEL attribute cicd.pipeline.tekton.dev.action.name

custom-tracing-label-domain: "cicd.pipeline.tekton.dev"
custom-tracing-label.action.name: "delivery.tekton.dev/action"  
custom-tracing-label.application: "delivery.tekton.dev/application"
custom-tracing-label.component: "delivery.tekton.dev/component"

Backwards compatibility

A few points regarding the backwards compatibility of the change

• The attributes were added by Konflux-CI developers
• Their documentation was never published (Docs: pipelines-as-code.com docs out of date #2902)
• No other (open source) project appears to reference these labels besides Konflux: GH search for delivery.tekton.dev
• Konflux-CI seems to change the delivery.tekton.dev labels to come from appstudio.openshift.io anyway: PVO11Y-5384 Enable delivery tracing in staging redhat-appstudio/infra-deployments#13238

[aThorp96]

CC @chmouel

CC @ci-operator since you contributed the otel tracing and these labels, I wanted to get your thoughts on this. Would it satisfy your needs?

[ci-operator]

Which labels the values come from is already configurable in the ConfigMap, using tracing-label-{action,application,component}. delivery.tekton.dev/* are just the defaults. See the docs on configuring label sourced attributes.

Only the attribute names are fixed - cicd.pipeline.action.name comes from OTel semconv, and delivery.tekton.dev.{application,component} match what integration-service and release-service emit, so you can query all three services with the same attribute names.

Are you asking to make the attribute names configurable too? If it's just the label names, that already exists. Or is this more about changing how those labels are configured - if that's the case, then this does make plenty of sense, as this regex-derived schema allows more flexibility

[aThorp96]

@ci-operator There's three pieces to this:

1. The span attribute names (e.g. delivery.tekton.dev.*, or cicd.pipeline.action.*)
2. The labels (e.g. appstudio.openshift.io/foo, delivery.tekton.dev/*, etc)
3. The "application", "component", and "action" concepts

For 1) delivery.tekton.dev doesn't match any Tekton convention or domain. Integration Service and Release Service are not part of Tekton, and so to me it adds confusion for Tekton to embed this convention. Similarly for 3), application, component, and action are not concepts in Tekton or Pipelines as Code. I would have similar concerns if PaC had Tack domain embedded in its configuration, like tracing-label-knot-id. I'm not against PaC injecting the attributes from the PLR labels, but PaC is an independent project upstream from Konflux so it's not the right place for the downstream attributes to be enumerated.

With respect to 2), I like that these are configurable and that makes sense to me. I think that's the right approach, and I'd like to extend the configuration further to allow downstream consumers like Release Service and Konflux to configure these types of labels (and more) without depending on upstream applications to embed the downstream models. IMO the configuration should treat 1) and 3) as unstructured, custom attributes with no details provided by PaC.

is this more about changing how those labels are configured

It would be how both the labels and the annotations are attribute names are configured. PaC wouldn't know or care about application, component, action, delivery.tekton.dev, or cicd.pipeine.action. That would all come from the configuration managed by the team that does care about those things.

[ci-operator]

I largely agree - the concepts aren't exclusively Konflux's, which is exactly why #2605 dropped the appstudio.openshift.io/* labels during review and used delivery.tekton.dev instead: a name belonging to no vendor and no single repo. ADR-0062 decides the attribute set; PaC, integration-service, and release-service just agree on the keys so a query means the same thing against all three.

Where I'd draw the line on configurability: cicd.pipeline.action.name is OTel semconv, and semconv names should stay fixed - a fixed name for a shared concept is the entire point of semconv. Only the invented delivery.tekton.dev.{application,component} keys have no standard behind them, so making those configurable makes sense.

That also gives a clean rule for any future attribute: semconv name where one exists, Tekton's convention where it has one, configurable for the rest. If that works for you I'm on board - today's mapping just needs to stay expressible in the new configuration.

[aThorp96]

Where I'd draw the line on configurability: cicd.pipeline.action.name is OTel semconv, and semconv names should stay fixed

Ack. Found the docs on the semconv and that makes sense to me. Knowing that, I think that attribute makes sense to be treated differently anyway since it has an always-valid default value of RUN (if a pipelineRun is started). We should probably fallback to setting it to be RUN when a custom label isn't configured and/or found.

today's mapping just needs to stay expressible in the new configuration.

Sounds like we're in agreement! 🙏🏼 👍🏼

[aThorp96]

I was looking into the OTel semconv and found a few other things which may be related here. It looks like delivery.tekton.dev.pipelinerun_uid would be appropriately moved to cicd.pipeline.run.id. For delivery.tekton.dev.result_message, there isn't a semconv for the complete error message, but since an error status is put in cicd.pipeline.result and the error category is supposed to be put in error.type, I wonder if cicd.pipeline.result.message or error.message would be the appropriate attribute? WDYT?

[aThorp96]

Actually it looks like error.message was part of semconv but has been deprecated:

Use domain-specific error message attribute. For example, use feature_flag.error.message for feature flag errors.

In that case, we can put it in a domain-specific error message attribute cicd.pipeline.error_message.

I think the following list is complete:

description	attribute before	value source before	attribute after	value source after
move pipelinerun id to semconv key	delivery.tekton.dev.pipelinerun_uid	PipelineRun UID	cicd.pipeline.run.id	PipelineRun UID
remove irrelevant domain, default pipeline action	cicd.pipeline.action.name	label delivery.tekton.dev/action, configured with tracing-label-action, or none	cicd.pipeline.action.name	label from config tracing-label-action or "RUN"
remove irrelevant domain. Support via custom attribute config	delivery.tekton.dev.application	label delivery.tekton.dev/application, configured with tracing-label-application, or none	configured with custom-tracing-label.delivery.tekton.dev.application or none	label from config value
remove irrelevant domain. Support via custom attribute config	delivery.tekton.dev.component	label delivery.tekton.dev/component, configured with tracing-label-component, or none	configured with custom-tracing-label.delivery.tekton.dev.component or none	label from config value
remove irrelevant domain. Support via custom attribute config	delivery.tekton.dev.result_message	first failing taskrun message	cicd.pipeline.error_message	first failing taskrun message
New custom attribute support	NA	NA	*, from config key custom-tracing-label.*	label from config value