Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for OpenTelemetry in addition to OpenTracing? #5883

Closed
hairyhenderson opened this issue Jul 10, 2020 · 33 comments
Closed

Support for OpenTelemetry in addition to OpenTracing? #5883

hairyhenderson opened this issue Jul 10, 2020 · 33 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@hairyhenderson
Copy link

The OpenTracing support is super useful, however recently OpenTracing has merged with the OpenTelemetry project, and it seems like many more integrations will be possible.

To my knowledge there doesn't yet exist an OpenTelemetry equivalent to the nginx-opentracing library yet, but the original author (@rnburn) is also active in https://github.com/open-telemetry/opentelemetry-cpp.

Given the lack of library it may be premature to ask for this in this repo, but I thought I would at least start the conversation here to see if there's any other interest.

For now my plan is to configure nginx-ingress-controller with the zipkin-collector-host parameter pointing at an instance of opentelemetry-collector, configured with the zipkin receiver.

The downside is that trace propagation won't be seamless - downstream services will have to be configured to support Zipkin (B3/X-B3-*) headers as well as the W3C Trace-Context (Traceparent) header.

/kind feature

@hairyhenderson hairyhenderson added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 10, 2020
@aledbf
Copy link
Member

aledbf commented Jul 13, 2020

@hairyhenderson interest sure, but I would prefer to know what is going to happen with nginx-opentracing first.
From the three plugins we use, only datadog is being actively maintained. Also, the three use different versions of the same libraries, something that introduces additional time to build the image. Errors like this one are hard to track too.
Link to the request to update zipkin-cpp-opentracing

@hairyhenderson
Copy link
Author

@aledbf ok... that may be orthogonal, though obviously it would be good to have the OpenTracing libraries up-to-date. I've noticed that the Jaeger client was recently updated, so it may be useful to bump that.

From an outsider's perspective it seems that a bit more effort is being put towards OpenTelemetry right now, which may explain the lull in OpenTracing development.

@aledbf
Copy link
Member

aledbf commented Jul 14, 2020

From an outsider's perspective it seems that a bit more effort is being put towards OpenTelemetry right now, which may explain the lull in OpenTracing development.

I agree with that. Now convince @rnburn to write a new nginx module 😉

@hairyhenderson
Copy link
Author

@aledbf I have no relationship with @rnburn and I'm sure he's under no obligation to listen to me 😉

There's also a feature request opentracing-contrib/nginx-opentracing#132 that may make it even easier to use ingress-nginx with opentelemetry, as it would solve the propagation issue. I'm just putting it here for reference.

@aledbf
Copy link
Member

aledbf commented Jul 15, 2020

There's also a feature request opentracing-contrib/nginx-opentracing#132

I am aware of that issue :) #5716

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 13, 2020
@project0
Copy link

project0 commented Nov 4, 2020

/remove-lifecycle stale

i hate this bots...

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 4, 2020
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 2, 2021
@lalitb
Copy link

lalitb commented Feb 12, 2021

/remove-lifecycle stale

Keeping alive to look forward on more interest in this requirement in coming days.

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 12, 2021
@dgzlopes
Copy link

Related: https://github.com/open-telemetry/opentelemetry-cpp-contrib/tree/main/instrumentation/nginx

@miry
Copy link
Contributor

miry commented Apr 13, 2021

With new version of Jaeger v0.7.0
jaegertracing/jaeger-client-cpp@c1d5795
it is possible to use W3C format in ingress-nginx with Jaeger tracer.

Ingress-nginx commit with new changes 71c8ef1

Example of config ingress-config:

---
apiVersion: v1
kind: ConfigMap
metadata:
  namespace: kube-system
  name: nginx-load-balancer-conf
data:
  enable-opentracing: "true"
  jaeger-service-name: nginx-ingress
  jaeger-collector-host: otel-collector.observability.svc.cluster.local
  jaeger-propagation-format: w3c
  worker-processes: "1"

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 12, 2021
@msschl
Copy link
Contributor

msschl commented Jul 12, 2021

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 12, 2021
@rikatz
Copy link
Contributor

rikatz commented Aug 12, 2021

@msschl can you confirm comment from @miry does not work your for you?

#5883 (comment)

Also there's some instrumentation module for OTEL right now, if someone proposes to work on a PR for this I can review it.

@dmathieu dmathieu mentioned this issue Sep 10, 2021
8 tasks
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 10, 2021
@dmathieu
Copy link
Contributor

/remove-lifecycle stale
I am working on this, though I'm current blocked by base image build timeouts.

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 12, 2021
@rikatz
Copy link
Contributor

rikatz commented Nov 12, 2021

/lifecycle active
@dmathieu let me try to help you unblocking this during this/next week

@k8s-ci-robot k8s-ci-robot added the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label Nov 12, 2021
@iamNoah1
Copy link
Contributor

/triage accepted

@k8s-ci-robot k8s-ci-robot added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Dec 15, 2021
@dmathieu
Copy link
Contributor

The latest development of this is #8013, which aims to bring the OpenTelemetry module as a sidecar.

@iamNoah1
Copy link
Contributor

iamNoah1 commented Dec 15, 2021

@dmathieu can consider this then as being obsolete with the implementation of #8013 ?

@dmathieu
Copy link
Contributor

Even with that PR, we will need #7621 (rebased and probably adapted) to have actual support.
I wouldn't consider this issue obsolete, as it tracks the need for the feature.

@cskinfill
Copy link
Contributor

@rcjsuen
Copy link

rcjsuen commented Dec 23, 2021

@cskinfill
Copy link
Contributor

nice! so any more progress being made on support for OpenTelemetry? Looks like #7621 is staled???

@dmathieu
Copy link
Contributor

Ongoing work is happening in #8013.

@strongjz
Copy link
Member

strongjz commented Mar 1, 2022

@dmathieu
Copy link
Contributor

dmathieu commented Mar 2, 2022

Sorry about that @strongjz. The nginx version was changed as a PR feedback, and I missed the checksum. #8286 fixes it.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. and removed lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. labels May 31, 2022
@dmathieu
Copy link
Contributor

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 31, 2022
@thomaschaaf
Copy link

Is this resolved with the latest merged PR #8719?

@Tobrek
Copy link
Contributor

Tobrek commented Jul 6, 2022

No. At least PR #8735 is still missing, to get a working open telemetry solution (using the sidecar). And after merging #8735 there is no specific configuration of the used open telemetry nginx module possible - enable or disable it at all. Maybe there will be am additional pull request to enable the detail configuration for the module.

@esigo
Copy link
Member

esigo commented Sep 30, 2022

WIP in #9016.
This issue probably can be closed.

@hairyhenderson
Copy link
Author

#9016 is a duplicate of this, but I'm fine closing this in favour of that one. I no longer use ingress-nginx anyway so I'm happy receiving fewer notifications 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests