Different enum value for codes.Code than proto (see OK, Error)

[bogdandrutu]

The otel-go defines:
https://github.com/open-telemetry/opentelemetry-go/blob/main/codes/codes.go#L23

The proto defines:
https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/trace/v1/trace.proto#L312

I think the conversion to from codes.Code to proto will be extremely fragile.

[MrAlias]

The conversion function we use is designed to avoid this problem.

opentelemetry-go/exporters/otlp/otlptrace/internal/tracetransform/span.go

Lines 131 to 144 in 0a56233

	// status transform a span code and message into an OTLP span status.
	func status(status codes.Code, message string) *tracepb.Status {
	var c tracepb.Status_StatusCode
	switch status {
	case codes.Error:
	c = tracepb.Status_STATUS_CODE_ERROR
	default:
	c = tracepb.Status_STATUS_CODE_OK
	}
	return &tracepb.Status{
	Code: c,
	Message: message,
	}
	}

Having a translation that directly converts the underlying integer value to the the other type will indeed be brittle and would couple the two projects unnecessarily. The design of that conversion function was intended to never introduce this type of error.

I'm not sure we should try to synchronize the enum ordering. It will add coupling between the two projects which is additional unneeded work, and it will inevitably result in bugs. Instead I think we should continue to convert by directly using the named variable types that actually have semantic meaning.

[bogdandrutu]

But as you can see in #2099 the conversion is wrong because of that exact reason you mentioned :)

[MadVikingGod]

I think that we should have an expectation that you need to go through the exporters API to convert between these different representations.

The solutions I see right now are:

1. Have no relation between the different "enums". This means that while you can cast them you will probably get the wrong result, setting the expectation to not do so now or in the future. Instead you have to use the API
2. Have the enums line up. This allows for direct casting, eg tracepb.Status_StatusCode(status), but that adds a burden on us to make sure our internal types forever line up with the otlp types. And the reason we use internal types is so that we have some ability to change them as we need to. We would also still need to maintain these conversion APIs where we could get this wrong.
3. Use the proto definition. This is a non-starter after the great pains we have taken to keep protobufs within their own areas.

All that said, I wouldn't want to change the API unless there was some really compelling reason to.

[bogdandrutu]

@MadVikingGod my main reason (with a clear proof) is that keeping them independent of the OTLP adds bugs (see the issue). So having the numbers aligned with the proto is a win (I think all the other languages do that).

[MrAlias]

Community consensus from last SIG meeting was to only couple these types with the explicit conversion functions that we offer and to not have the underlying types synchronized.