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

Upgrade internal use of tracing to OpenTelemetry #3381

Closed
1 task done
Tracked by #3362
yurishkuro opened this issue Nov 8, 2021 · 15 comments
Closed
1 task done
Tracked by #3362

Upgrade internal use of tracing to OpenTelemetry #3381

yurishkuro opened this issue Nov 8, 2021 · 15 comments

Comments

@yurishkuro
Copy link
Member

yurishkuro commented Nov 8, 2021

This is aimed as an intern project for the LFX Mentorship. Please refer to #4453. Apply here.

Historically, the Jaeger backend used the OpenTracing API, with Jaeger's own Go SDK jaeger-client-go, for instrumenting its own internals for distributed tracing. Since Jaeger's SDKs have been deprecated (#3362), we want to upgrade the Jaeger backend to use the OpenTelemetry tracing API and SDK directly.

Expected Outcome:

  • Replace the use of OpenTracing API with OpenTelemetry
  • Remove jaeger-client-go and jaeger-lib as dependencies
  • Remove opentracing-go and opentracing-contrib/* as dependencies
  • Switch to standard OpenTelemetry instrumentation libraries where available (e.g. for HTTP, gRPC)
  • Rethink/rework crossdock integration tests to test end-to-end flow with OpenTelemetry data
  • Publish a blog post on medium.com/jaegertracing documenting the experience

Proposed approach

I don't think using OTEL-OT Bridge makes sense for this project, since it will create an extra work for adding the bridge, but will not make the actual migration any easier. Instead, I would do this in stages:

  • Right now various components of the backend take opentracing.Tracer as argument. As a first step, we create a wrapper object that will contain that same tracer, and pass the wrapper object to components. This will be one-time large refactoring but without any functional changes.
  • Once the first step is done, we can add OTEL Tracer to the same wrapper object.
  • Then start changing components one by one by switching them to use OTEL instrumentation.
  • When all of them have been migrated, we can remove the OpenTracing part.

Some related issues:

@yurishkuro yurishkuro added the help wanted Features that maintainers are willing to accept but do not have cycles to implement label Nov 8, 2021
@yurishkuro
Copy link
Member Author

Filed as a project for LFX Mentorship cncf/mentoring#947

@swastik959
Copy link

@yurishkuro hi I am new to this org and want to work on this issue I have worked on some kubernetes sigs and has a knowledge of golang

@yurishkuro
Copy link
Member Author

@swastik959 I prefer to reserve this project for the LFX Mentorship program (otherwise I won't have a project there). You can either apply there or please pick other help-wanted issues in this repo.

@swastik959
Copy link

@yurishkuro can you guide me through its process of getting selected

@yurishkuro
Copy link
Member Author

I don't know all details yet, I assume the process will be documented here https://github.com/cncf/mentoring/tree/main/programs/lfx-mentorship/2023/02-Jun-Aug

@yurishkuro
Copy link
Member Author

@octonawish-akcodes
Copy link

I am interested in this issue under lfx mentorship

@rhzx3519
Copy link

rhzx3519 commented May 2, 2023

@yurishkuro Hi, I'm very interested in this task, and I'm familiar with Golang and have contributed to some cloud-native projects.

@Shubham4359
Copy link

Hi @yurishkuro, i would love to work on this issue during the upcoming LFX mentorship.
I have previously worked in Golang and have also Contributed to other CNCF organisations Like Thanos.
Can you please provide some starting material to work on this issue also any other things to keep in mind.
Thanks

@Swapnil-2502
Copy link

Filed as a project for LFX Mentorship cncf/mentoring#947

Yeah I just saw it will be applying for the LFX Mentorship.

@shashankiitbhu
Copy link

Hi, I am very interested in this task and would like to work on this under LFX mentorship program.

@yurishkuro
Copy link
Member Author

Applicants interested in this project: please see #4453

@haanhvu
Copy link
Contributor

haanhvu commented May 15, 2023

For anyone looking for guide/resources to study this project:

@yurishkuro
Copy link
Member Author

@afzal442 will be working on this as part of LFX Mentorship program.

@yurishkuro yurishkuro added mentorship and removed help wanted Features that maintainers are willing to accept but do not have cycles to implement labels Jun 14, 2023
yurishkuro pushed a commit that referenced this issue Jun 14, 2023
…TEL in the future [part 1] (#4529)

## Which problem is this PR solving?
Part of #3381 

## Short description of the changes
- This refactors the code by introducing a new wrapper object `JTracer`
to wrap that interface

Signed-off-by: afzal442 <afzal442@gmail.com>
yurishkuro pushed a commit that referenced this issue Jun 20, 2023
## Which problem is this PR solving?

Related to #3380 
Part of #3381 

- This adds `OTEL tracerProvider` and removes `OT tracer` for Redis
service hotROD app

Before:

![r2](https://github.com/jaegertracing/jaeger/assets/11625672/446d03b9-2da0-47b2-9312-b8bc86c7e78a)


After:

![r1](https://github.com/jaegertracing/jaeger/assets/11625672/ed3f2ca6-7a01-4ab2-a13c-56c1f3388fe6)

---------

Signed-off-by: afzal442 <afzal442@gmail.com>
Co-authored-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
kjschnei001 pushed a commit to kjschnei001/jaeger that referenced this issue Jun 29, 2023
…TEL in the future [part 1] (jaegertracing#4529)

## Which problem is this PR solving?
Part of jaegertracing#3381

## Short description of the changes
- This refactors the code by introducing a new wrapper object `JTracer`
to wrap that interface

Signed-off-by: afzal442 <afzal442@gmail.com>
Signed-off-by: KevinSchneider <kevin.schneider@target.com>
kjschnei001 pushed a commit to kjschnei001/jaeger that referenced this issue Jun 29, 2023
## Which problem is this PR solving?

Related to jaegertracing#3380
Part of jaegertracing#3381

- This adds `OTEL tracerProvider` and removes `OT tracer` for Redis
service hotROD app

Before:

![r2](https://github.com/jaegertracing/jaeger/assets/11625672/446d03b9-2da0-47b2-9312-b8bc86c7e78a)

After:

![r1](https://github.com/jaegertracing/jaeger/assets/11625672/ed3f2ca6-7a01-4ab2-a13c-56c1f3388fe6)

---------

Signed-off-by: afzal442 <afzal442@gmail.com>
Co-authored-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
Signed-off-by: KevinSchneider <kevin.schneider@target.com>
yurishkuro pushed a commit that referenced this issue Jul 2, 2023
## Which problem is this PR solving?
Related to #3380
Part of #3381

## Short description of the changes
- This PR adds OTEL instrumentation to customer svc

---------

Signed-off-by: Afzal Ansari <afzal442@gmail.com>
afzal442 added a commit to Cloud-Hacks/jaeger that referenced this issue Jul 10, 2023
## Which problem is this PR solving?
Related to jaegertracing#3380
Part of jaegertracing#3381

## Short description of the changes
- This PR adds OTEL instrumentation to customer svc

---------

Signed-off-by: Afzal Ansari <afzal442@gmail.com>
yurishkuro pushed a commit that referenced this issue Jul 20, 2023
## Which problem is this PR solving?
* Part of #3381 
* This PR adds OTEL Provider to jtracer pkg

## Short description of the changes
- Upgrades jaeger tracing to support `otel tracer`

---------

Signed-off-by: Afzal Ansari <afzal442@gmail.com>
Signed-off-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
Co-authored-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
yurishkuro pushed a commit that referenced this issue Jul 29, 2023
#4595)

## Which problem is this PR solving?
* Part of #3381 
* This PR adds OTEL Provider to metrics reader component

## Short description of the changes
- Replaces OT tracer with OTEL tracer to support jtracer pkg

---------

Signed-off-by: Afzal Ansari <afzal442@gmail.com>
yurishkuro pushed a commit that referenced this issue Jul 30, 2023
## Which problem is this PR solving?
* Part of #3381 
* This PR adds OTEL Provider to es-spanstore component

## Short description of the changes
- Replaces OT tracer with OTEL tracer to support jtracer pkg

---------

Signed-off-by: Afzal Ansari <afzal442@gmail.com>
Signed-off-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
Co-authored-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
yurishkuro pushed a commit that referenced this issue Aug 3, 2023
)

## Which problem is this PR solving?
* Part of #3381 
* This PR adds `otel trace` spans type to span model

## Short description of the changes
- Replace OT trace with `otel trace` spans type i.e. `ext.SpanKind..`
with `trace.SpanKind..`

---------

Signed-off-by: Afzal Ansari <afzal442@gmail.com>
Signed-off-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
Co-authored-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
yurishkuro pushed a commit that referenced this issue Aug 4, 2023
…e plugin (#4611)

## Which problem is this PR solving?
* Part of #3381 
* This PR adds `otelgrpc` plugin to storage

## Short description of the changes
- Replaces `otgrpc` plugin storage with otel grpc interceptors

---------

Signed-off-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
Co-authored-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
yurishkuro pushed a commit that referenced this issue Aug 4, 2023
## Which problem is this PR solving?
* Part of #3381 
* This PR adds `otel trace` spans type to span model

## Short description of the changes
- Replace OT const with `otel trace` spans const i.e. `ext.SpanKind..`
with `trace.SpanKind..`

---------

Signed-off-by: Afzal Ansari <afzal442@gmail.com>
Signed-off-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
Co-authored-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
yurishkuro pushed a commit that referenced this issue Aug 7, 2023
## Which problem is this PR solving?
* Part of #3381 
* This PR adds `otel trace` spans const type to domain model

## Short description of the changes
- Replace OT const with `otel trace` spans const i.e. `ext.SpanKind..`
with `trace.SpanKind..`

Signed-off-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
Co-authored-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
yurishkuro pushed a commit that referenced this issue Aug 12, 2023
## Which problem is this PR solving?
* Part of #3381 
* This PR adds `otel` traces and removes deprecated `jaeger-client-go`
api

## Short description of the changes
- Replace OT tracer with OTEL

---------

Signed-off-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
Co-authored-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
yurishkuro pushed a commit that referenced this issue Aug 12, 2023
## Which problem is this PR solving?
* Part of #3381 
* This PR adds `otel` traces and const to hotROD test file

## Short description of the changes
- Replace OT const with `otel trace` spans const i.e. `ext.SpanKind..`
with `trace.SpanKind..` and its trace API

---------

Signed-off-by: Afzal Ansari <afzal442@gmail.com>
Signed-off-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
Co-authored-by: Afzal <94980910+afzalbin64@users.noreply.github.com>
yurishkuro pushed a commit that referenced this issue Aug 13, 2023
<!--
Please delete this comment before posting.

We appreciate your contribution to the Jaeger project! 👋🎉

Before creating a pull request, please make sure:
- Your PR is solving one problem
- You have read the guide for contributing
- See
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING.md
- You signed all your commits (otherwise we won't be able to merge the
PR)
- See
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md#certificate-of-origin---sign-your-work
- You added unit tests for the new functionality
- You mention in the PR description which issue it is addressing, e.g.
"Resolves #123"
-->

## Which problem is this PR solving?
* Part of #3381 
* This PR removes `opentracing pkg` from jtracer pkg and go.mod file

## Short description of the changes
- Removes `opentracing pkg` from `jaeger module` and modifies README
file to rmv OT support.

---------

Signed-off-by: Afzal Ansari <afzal442@gmail.com>
yurishkuro pushed a commit that referenced this issue Sep 4, 2023
## Which problem is this PR solving?
* Part of #3381 
* This PR removes `jaeger-client` pkg constant and replaces it with
const variable

---------

Signed-off-by: afzal442 <afzal442@gmail.com>
yurishkuro added a commit that referenced this issue Sep 4, 2023
## Which problem is this PR solving?
- Part of #3381

## Description of the changes
- Remove internal unused package `jlibadapter`
- Remove unnecessary dependency on jaeger-lib in the tests

## How was this change tested?
- Unit tests

Signed-off-by: Yuri Shkuro <github@ysh.us>
@yurishkuro
Copy link
Member Author

This can be considered complete. We still import jaeger-client-go, but only in tests.

$ grx jaeger-client-go .
./crossdock/services/tracehandler_test.go:31:	"github.com/uber/jaeger-client-go"
./crossdock/services/tracehandler.go:29:	"github.com/uber/jaeger-client-go"
./cmd/collector/app/handler/http_thrift_handler_test.go:32:	jaegerClient "github.com/uber/jaeger-client-go"
./cmd/collector/app/handler/http_thrift_handler_test.go:33:	"github.com/uber/jaeger-client-go/transport"
./go.mod:50:	github.com/uber/jaeger-client-go v2.30.0+incompatible

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

No branches or pull requests

8 participants