✨: Add OpenTelemetry Tracing Support#6002
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @AnmolxSingh. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
|
|
||
| //creating span using OpenTelemetry SDK | ||
| tr := otel.Tracer("awscluster_controller") | ||
| ctx, span := tr.Start(ctx, "Reconcile() awscluster_controller.go") |
There was a problem hiding this comment.
It would be good to follow a naming convention. Perhaps it could be:
AWSClusterReconciler.Reconcile
| log := logger.FromContext(ctx) | ||
|
|
||
| //creating span using OpenTelemetry SDK | ||
| tr := otel.Tracer("awscluster_controller") |
There was a problem hiding this comment.
It might be good to wrap this into our own function that returns a tracer. This would force consumers to supply specific values.
There was a problem hiding this comment.
This would allow us to add attributes etc
There was a problem hiding this comment.
For example, in one of the projects i added otel to:
ctx, logger, span := telemetry.StartSpanWithLogger(ctx,
"controllers.MyController.Reconcile",
telemetry.KVP("namespace", req.Namespace),
telemetry.KVP("name", req.Name),
telemetry.KVP("kind", mytypev1.MyKind),
)
defer span.End()There was a problem hiding this comment.
I have added a simple function as of now. I can add more values to it as per the requirement.
WIP: Add OpenTelemetry Tracing Support
|
I have removed the aws api calls in this pr. I will open a separate pr for that. |
|
/test pull-cluster-api-provider-aws-test |
6d7a05d to
4dcb144
Compare
|
@richardcase I think this pull request is now ready to be merged |
WIP: Add OpenTelemetry Tracing Support|
@AnmolxSingh - it looks like this contains merge commits: Could you rebase instead? |
- add tracing helper package - instrument reconciliation flows - update tests for traced contexts
4dcb144 to
5ab7190
Compare
|
@AnmolxSingh - i think this looks good. Can you rebase and squash your commits? We can then get it merged. |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dlipovetsky The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Adding OpenTelemetry Tracing support for Kubernetes CAPA
More details are shared in #5982
Fixes #2178
Special notes for your reviewer:
AI Usage:
None
Checklist:
Release note: