Migrate jaeger to otel API #7547
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,15 +18,19 @@ package tracing | |
|
|
||
| import ( | ||
| "context" | ||
| "encoding/base64" | ||
| "fmt" | ||
| "net/url" | ||
|
|
||
| "github.com/tektoncd/pipeline/pkg/apis/config" | ||
| "go.opentelemetry.io/otel" | ||
| "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" | ||
| "go.opentelemetry.io/otel/propagation" | ||
| "go.opentelemetry.io/otel/sdk/resource" | ||
| tracesdk "go.opentelemetry.io/otel/sdk/trace" | ||
| semconv "go.opentelemetry.io/otel/semconv/v1.12.0" | ||
| "go.opentelemetry.io/otel/trace" | ||
| "go.opentelemetry.io/otel/trace/embedded" | ||
| "go.opentelemetry.io/otel/trace/noop" | ||
| "go.uber.org/zap" | ||
| corev1 "k8s.io/api/core/v1" | ||
|
|
@@ -35,15 +39,14 @@ import ( | |
| ) | ||
|
|
||
| type tracerProvider struct { | ||
| embedded.TracerProvider | ||
|
|
||
| service string | ||
| provider trace.TracerProvider | ||
| cfg *config.Tracing | ||
| username string | ||
| password string | ||
| logger *zap.SugaredLogger | ||
| } | ||
|
|
||
| func init() { | ||
|
|
@@ -150,12 +153,32 @@ func createTracerProvider(service string, cfg *config.Tracing, user, pass string | |
| if !cfg.Enabled { | ||
| return noop.NewTracerProvider(), nil | ||
| } | ||
| u, err := url.Parse(cfg.Endpoint) | ||
|
There was a problem hiding this comment. @kmjayadeep question: why do we need the additional configuration and the config-tracing configmap when it seems like otel can be configured with env vars (https://pkg.go.dev/go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp#pkg-overview)? Should we just have one way to configure tracing? (the env vars seem to support more options too e.g. client certs etc. vs the config map which only supports basic auth) There was a problem hiding this comment. Hi @dibyom, we have used env vars initially. Then we decided to move to configmap to make it consistent with how other configurations are managed in the project. Also, env variables requires the user to restart the controller when the configuration needs to be changed. |
||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| opts := []otlptracehttp.Option{ | ||
| otlptracehttp.WithEndpoint(u.Host), | ||
| otlptracehttp.WithURLPath(u.Path), | ||
| } | ||
|
|
||
| if u.Scheme == "http" { | ||
| opts = append(opts, otlptracehttp.WithInsecure()) | ||
| } | ||
|
|
||
| if user != "" && pass != "" { | ||
| creds := fmt.Sprintf("%s:%s", user, pass) | ||
| enc := base64.StdEncoding.EncodeToString([]byte(creds)) | ||
| o := otlptracehttp.WithHeaders(map[string]string{ | ||
| "Authorization": fmt.Sprintf("Basic %s", enc), | ||
| }) | ||
| opts = append(opts, o) | ||
| } | ||
|
Comment on lines
+170
to
+177
There was a problem hiding this comment. Is it an option to have a user with empty password? There was a problem hiding this comment. What I found during testing with jaeger and nginx is that if the username or password is configured to be empty, the server just ignores the whole authorization header. So no need to pass the header in code. |
||
|
|
||
| ctx := context.Background() | ||
| exp, err := otlptracehttp.New(ctx, opts...) | ||
|
|
||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jaeger exporter expected a complete URL path like
http://website.com:port/path, while otlp exporter accepts host and path as separate arguments. This URL parsing is added to ensure that user experience has not changed.