Skip to content
This repository has been archived by the owner on Dec 21, 2023. It is now read-only.

Switch from sh.keptn.context to TraceContext/ParentContext (transparent) #1614

Closed
2 tasks
christian-kreuzberger-dtx opened this issue Apr 7, 2020 · 14 comments
Closed
2 tasks

Comments

@christian-kreuzberger-dtx
Copy link
Member

christian-kreuzberger-dtx commented Apr 7, 2020

Keptn should support w3c tracecontext.

Research

Prototype

As a POC implementation, an existing keptn endpoint shall be used to demonstrate the usage of trace context.
The interaction between the keptn cli and the keptn endpoint shall be shown.

Tasks

  • implement poc
  • Define additional issues that came up during investigation
@christian-kreuzberger-dtx christian-kreuzberger-dtx modified the milestone: 0.7.0 Apr 7, 2020
@stale
Copy link

stale bot commented Jun 7, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 7, 2020
@johannes-b johannes-b added this to the 0.8.0 milestone Jun 8, 2020
@stale stale bot removed the stale label Jun 8, 2020
@agrimmer agrimmer added the future We'll revisit this issue during release planning and evaluate label Jun 8, 2020
@johannes-b
Copy link
Member

An extension of the CloudEvents focuses on exactly this topic. Let's have a look at: https://github.com/cloudevents/spec/blob/master/extensions/distributed-tracing.md when working on that issue.

@johannes-b johannes-b removed the kep:06 label Oct 29, 2020
@johannes-b johannes-b added the ready-for-refinement Issue is relevant for the next backlog refinment label Nov 17, 2020
@johannes-b johannes-b added research next-sprint Items that should be discussed and implemented in the next sprint and removed future We'll revisit this issue during release planning and evaluate ready-for-refinement Issue is relevant for the next backlog refinment labels Dec 1, 2020
@johannes-b
Copy link
Member

How is this issue related to: #2422 ?

@christian-kreuzberger-dtx
Copy link
Member Author

#2422 is meant to allow capturing certain custom metrics (e.g., git pulls in prometheus-service) that would allow us to track Keptns performance, while #1614 (this issue) is meant to introduce tracecontext etc...

@johannes-b johannes-b added this to Backlog in Working items via automation Dec 9, 2020
@johannes-b johannes-b moved this from Backlog to Assigned and committed in Working items Dec 10, 2020
@johannes-b johannes-b removed the next-sprint Items that should be discussed and implemented in the next sprint label Dec 10, 2020
@warber
Copy link
Contributor

warber commented Dec 17, 2020

After a very helpful discussion with @slinkydeveloper, it seems like the distributed tracing extension offered by the cloudevents spec is not that useful and misunderstood by many people. First of all, thought the name would suggest it, the distributed tracing extension is not really usable for doing distributed tracing. It does not even relate to the use cases defined in the W3C standard. A lot of the confusion originates from the fact that the extension spec does follow the naming and format of the W3C Trace Context standard. There is quite a chance that this feature will be removed and reworked in the cloudevents specification (see: cloudevents/spec#751 for more information)
As for now using the distributed tracing feature is equivalent to renaming the shkeptncontext field to traceparent and change its format to the one mentioned in the W3C standard. (@johannes-b @agrimmer should we still do that? what's the benefit? I believe it brings also a lot of confusion into the keptn project. And why should the keptn context follow the format of W3C trace context? )

To truly introduce distributed tracing we should use the tools and APIs offered by opentelemetry. (@johannes-b @agrimmer follow up issue?)

@warber warber moved this from Assigned and committed to In progress in Working items Dec 17, 2020
@johannes-b
Copy link
Member

Thanks for reaching out to the CloudEvents team. It is unfortunate that the extension does not help us and is still subject to change. Consequently, don't let's continue research this direction.

However, I would suggest figuring out how to make use of the official APIs offered by OpentTelementry as part of this issue.

I agree with you that replacing shkeptncontext with traceparent is quite a big change. But what do you think about adding traceparent as a header attribute to introduce it to Keptn CloudEvents?

@warber
Copy link
Contributor

warber commented Dec 18, 2020

But what do you think about adding traceparent as a header attribute to introduce it to Keptn CloudEvents?

Yes, we could do that. This basically just mean: introduce OpenTelemetry into Keptn.

However, it is still a bit unclear about what we want to achieve. (1) Is it the goal to introduce distributed tracing into the architecture of keptn. Meaning, being able to trace all calls between our microservices using the capabilities of OpenTelemetry?
(2) Or, do we just want to correlate our CloudEvents, i.e., being able to see which CloudEvents belong together in a trace of an e2e usecase (user triggers some action, and we want to easily see the chain of cloud events that belong to this action)

IMO, these are two different things, (1) means introducing OpenTelemetry, while (2) means adding information to our CloudEvents to correlate them. It does not mean that (1) and (2) cant be solved using the same technology, though ;)

@warber warber moved this from In progress to Backlog in Working items Dec 21, 2020
@johannes-b johannes-b added the ready-for-refinement Issue is relevant for the next backlog refinment label Jan 12, 2021
@warber warber removed their assignment Jan 13, 2021
@johannes-b johannes-b modified the milestones: 0.8.0, 0.8.1 Feb 18, 2021
@johannes-b johannes-b removed this from Backlog in Working items Feb 18, 2021
@johannes-b johannes-b removed the ready-for-refinement Issue is relevant for the next backlog refinment label Feb 22, 2021
@johannes-b johannes-b removed this from the 0.8.1 milestone Feb 22, 2021
@stale
Copy link

stale bot commented Jun 2, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 2, 2021
@johannes-b johannes-b removed the stale label Jun 7, 2021
@stale
Copy link

stale bot commented Aug 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale
Copy link

stale bot commented Oct 11, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 11, 2021
@thisthat thisthat removed the stale label Oct 11, 2021
@stale
Copy link

stale bot commented Jan 3, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 3, 2022
@thisthat thisthat removed the stale label Jan 5, 2022
@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 16, 2022
@thisthat thisthat removed the stale label Apr 21, 2022
@stale
Copy link

stale bot commented Jul 10, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jul 10, 2022
@thisthat
Copy link
Member

Semantically, the sh.keptn.context is different than the trace context. Hence we cannot replace one with another.
Trace Context can be propagated via CloudEvent natively now with this extension: https://github.com/cloudevents/spec/blob/v1.0.2/cloudevents/extensions/distributed-tracing.md

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants