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

Zipkin exporter and collector do not agree on wire representation of status codes #3287

Closed
mterwill opened this issue Oct 14, 2022 · 2 comments · Fixed by #3340
Closed

Zipkin exporter and collector do not agree on wire representation of status codes #3287

mterwill opened this issue Oct 14, 2022 · 2 comments · Fixed by #3340
Labels
bug Something isn't working help wanted Extra attention is needed pkg:exporter:zipkin Related to the Zipkin exporter package

Comments

@mterwill
Copy link

mterwill commented Oct 14, 2022

Description

The Zipkin exporter and collector do not agree on wire representation of status codes. Please let me know if this issue should be filed in open-telemetry/opentelemetry-collector-contrib instead.

The Zipkin exporter serializes status codes as Unset, Error, and Ok:

if data.Status().Code != codes.Unset {
m["otel.status_code"] = data.Status().Code.String()
}

var codeToStr = map[Code]string{
Unset: "Unset",
Error: "Error",
Ok: "Ok",
}

However, the collector expects STATUS_CODE_UNSET, STATUS_CODE_OK, or STATUS_CODE_ERROR, see here.

Environment

opentelemetry-go v1.11.0
opentelemetry-collector-contrib v0.62.0

Steps To Reproduce

Using Grafana Tempo's local example:

package main

import (
	"context"
	"log"

	"go.opentelemetry.io/otel"
	"go.opentelemetry.io/otel/codes"
	"go.opentelemetry.io/otel/exporters/zipkin"
	sdktrace "go.opentelemetry.io/otel/sdk/trace"
)

func main() {
	ctx := context.Background()

	exporter, err := zipkin.New("")
	if err != nil {
		log.Fatalf("creating exporter: %s", err)
	}

	tp := sdktrace.NewTracerProvider(
		sdktrace.WithSampler(sdktrace.AlwaysSample()),
		sdktrace.WithBatcher(exporter),
	)
	defer tp.Shutdown(ctx)
	otel.SetTracerProvider(tp)

	_, span := otel.Tracer("foo").Start(ctx, "foo")
	log.Printf("trace ID: %s", span.SpanContext().TraceID())
	span.SetStatus(codes.Error, "something went wrong")
	span.End()
}

image

Expected behavior

Status is correctly passed between exporter and collector.

@mterwill mterwill added the bug Something isn't working label Oct 14, 2022
@mterwill mterwill changed the title Zipkin status codes Zipkin exporter and collector do not agree on wire representation of status codes Oct 14, 2022
@MrAlias
Copy link
Contributor

MrAlias commented Oct 14, 2022

Based on the specification the status value should be the ...

Name of the code, either OK or ERROR. MUST NOT be set if the code is UNSET.

It seems like we are not correctly setting it to an upper case value, but it also seems like the collector is incorrectly expecting it to be one of STATUS_CODE_UNSET, STATUS_CODE_OK, or STATUS_CODE_ERROR. I think fixes need to be applied on both sides.

@MrAlias
Copy link
Contributor

MrAlias commented Oct 14, 2022

@mterwill can you open an issue with the collector as well to track this parsing error?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed pkg:exporter:zipkin Related to the Zipkin exporter package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants