Zipkin V2 Trace Receiver doesn't parse tags like http.status_code

[chris-smith-zocdoc]

Zipkin tags/annotations is always a string and they must be parsed to successfully convert to the open census format.

Current the v1 and v2 zipkin tag/binary annotation parsers are separate code paths

V1 supports

• integers
• booleans
• span status from common tags

opentelemetry-collector/translator/trace/zipkin/zipkinv1_to_protospan.go

Lines 216 to 219 in 86e5805

	if iValue, err := strconv.ParseInt(binAnnotation.Value, 10, 64); err == nil {
	pbAttrib.Value = &tracepb.AttributeValue_IntValue{IntValue: iValue}
	} else if bValue, err := strconv.ParseBool(binAnnotation.Value); err == nil {
	pbAttrib.Value = &tracepb.AttributeValue_BoolValue{BoolValue: bValue}

opentelemetry-collector/translator/trace/zipkin/status_code.go

Lines 69 to 98 in 86e5805

	func (m *statusMapper) fromAttribute(key string, attrib *tracepb.AttributeValue) bool {
	switch key {
	case tracetranslator.TagZipkinCensusCode:
	m.fromCensus.codePtr = attribToStatusCode(attrib)
	return true
	case tracetranslator.TagZipkinCensusMsg:
	m.fromCensus.message = attrib.GetStringValue().GetValue()
	return true
	case tracetranslator.TagStatusCode:
	m.fromStatus.codePtr = attribToStatusCode(attrib)
	return true
	case tracetranslator.TagStatusMsg:
	m.fromStatus.message = attrib.GetStringValue().GetValue()
	return true
	case tracetranslator.TagHTTPStatusCode:
	codePtr := attribToStatusCode(attrib)
	if codePtr != nil {
	*codePtr = tracetranslator.OCStatusCodeFromHTTP(*codePtr)
	m.fromHTTP.codePtr = codePtr
	}
	case tracetranslator.TagHTTPStatusMsg:
	m.fromHTTP.message = attrib.GetStringValue().GetValue()
	}
	return false
	}

V2 only supports

• booleans (restricted formats)
• span status (only open census canonical code)

opentelemetry-collector/receiver/zipkinreceiver/trace_receiver.go

Lines 594 to 611 in 86e5805

	for key, value := range tags {
	// We did a translation from "boolean" to "string"
	// in OpenCensus-Go's Zipkin exporter as per
	// https://github.com/census-instrumentation/opencensus-go/blob/1eb9a13c7dd02141e065a665f6bf5c99a090a16a/exporter/zipkin/zipkin.go#L138-L155
	switch value {
	case "true", "false":
	amap[key] = &tracepb.AttributeValue{
	Value: &tracepb.AttributeValue_BoolValue{BoolValue: value == "true"},
	}
	default:
	amap[key] = &tracepb.AttributeValue{
	Value: &tracepb.AttributeValue_StringValue{
	StringValue: &tracepb.TruncatableString{Value: value},
	},
	}
	}
	}

opentelemetry-collector/receiver/zipkinreceiver/trace_receiver.go

Lines 488 to 529 in d86bcd0

	func extractProtoStatus(zs *zipkinmodel.SpanModel) *tracepb.Status {
	// The status is stored with the "error" key
	// See https://github.com/census-instrumentation/opencensus-go/blob/1eb9a13c7dd02141e065a665f6bf5c99a090a16a/exporter/zipkin/zipkin.go#L160-L165
	if zs == nil || len(zs.Tags) == 0 {
	return nil
	}
	canonicalCodeStr := zs.Tags["error"]
	message := zs.Tags["opencensus.status_description"]
	if message == "" && canonicalCodeStr == "" {
	return nil
	}
	code, set := canonicalCodesMap[canonicalCodeStr]
	if !set {
	// If not status code was set, then we should use UNKNOWN
	code = statusCodeUnknown
	}
	return &tracepb.Status{
	Message: message,
	Code: code,
	}
	}
	var canonicalCodesMap = map[string]int32{
	// https://github.com/googleapis/googleapis/blob/bee79fbe03254a35db125dc6d2f1e9b752b390fe/google/rpc/code.proto#L33-L186
	"OK": 0,
	"CANCELLED": 1,
	"UNKNOWN": 2,
	"INVALID_ARGUMENT": 3,
	"DEADLINE_EXCEEDED": 4,
	"NOT_FOUND": 5,
	"ALREADY_EXISTS": 6,
	"PERMISSION_DENIED": 7,
	"RESOURCE_EXHAUSTED": 8,
	"FAILED_PRECONDITION": 9,
	"ABORTED": 10,
	"OUT_OF_RANGE": 11,
	"UNIMPLEMENTED": 12,
	"INTERNAL": 13,
	"UNAVAILABLE": 14,
	"DATA_LOSS": 15,
	"UNAUTHENTICATED": 16,
	}

I think these code paths should be unified so they have the same behavior.

[Verlet64]

I'd be happy to pick this up, if that's okay?

[chris-smith-zocdoc]

I'm actually planning on working on this today @Verlet64 as its a requirement for my work

[kbrockhoff]

This fix needs to be coordinated with any fixes for #1138