Use semconv in integration tests

[carrbs]

Fixes: #630

This is a suggestion for possibly changing the scope of this ticket to start using the opentelemetry-go semconv definitions generated from semantic conventions defined by OpenTelemetry.

The PR introduces a few helper methods that convert attribute.KeyValues into jaeger.Tags. It further exercises these functions several times in one of the integration test files to give an example of usage.

If this seems like an appropriate direction to take, I can finalize the PR (likely just applying the approach to the rest of this test file), and assist with rescoping the issue and opening tickets to utilize this pattern in other files.

A few thoughts if we move forward on this:

1. I wasn't sure how to handle a one of the tags (jaeger.Tag{Key: "span.kind", Type: "string", Value: "server"}) because it wasn't defined in the semantic-conventions.
2. If there's a more appropriate location for the helper methods let me know and I can move them.

[CLAassistant]

All committers have signed the CLA.

[mariomac]

Hi @carrbs ! Thank you for your contribution. We will review and discuss it with the team.

One advantage of keeping the tag names hardcoded in the tests is that we could detect any breaking change derived from the update of the OTEL libraries and properly fix or document it.

Keeping the same functions to generate the tag names and to check their values could cause that any breaking change in the OTEL library remains unnoticed to us and leaking undocumented breaking changes to our users.

[mariomac]

To minimize the use of generic helper files, I'd consider moving these functions to the test/integration/jaeger/jaeger.go files and rename them to something like jaeger.FromOTEL(kv attribute.KeyValue) jaeger.Tag

[carrbs]

I moved the functions and renamed them. if you prefer "FromOTEL" vs "FromOtel" let me know and I can make that change as well 👍.

[carrbs]

Hi @mariomac! 👋

I was also thinking about how we could best handle this. One thought I had was adding the stability as an attribute within the generated semconv. This could be utilized in a step during testing: if the stability is deprecated, tests could fail (or warn) triggering a task to update the name. Would this be a useful approach?

---

Regarding this:

One advantage of keeping the tag names hardcoded in the tests is that we could detect any breaking change derived from the update of the OTEL libraries and properly fix or document it.

Can you help me understand how the hardcoded names help to detect the breaking changes? When OTEL libraries are updated, the generated semconv is also updated. Deprecation in the generated files is scoped to the comments, for example:

// github.com/open-telemetry/opentelemetry-go/tree/main/semconv/v1.24.0/resource.go
// ...
// Span attributes used by non-OTLP exporters to represent OpenTelemetry Scope's concepts.
const (
	// Deprecated, use the `otel.scope.name` attribute.
	//
	// Type: string
	// RequirementLevel: Optional
	// Stability: deprecated
	// Examples: 'io.opentelemetry.contrib.mongodb'
	OtelLibraryNameKey = attribute.Key("otel.library.name")
	// Deprecated, use the `otel.scope.version` attribute.
	//
	// Type: string
	// RequirementLevel: Optional
	// Stability: deprecated
	// Examples: '1.0.0'
	OtelLibraryVersionKey = attribute.Key("otel.library.version")
)

[mariomac]

Hi @carrbs! The issue here es that even if we argue that we are compliant with the latest stable specifications of OTEL, any breaking change produced after upgrading (and they are frequent), could break the stored queries in the dashboards and alerts of our users.

Imagine we are using version X of OTEL libraries, and we use the unstable semconv.Example constant whose value is example. The users manually set this field name in their queries.

Then we update to the version Y of the libraries, which modified the semconv.Example value to request.example. The next Beyla release would break users' queries.

If we used the semconv.Example constant in our integration tests, this change could have been unnoticed to us, as both producer and tests values are updated automatically and the tests passes. The next Beyla release would introduce an unnoticed breaking change.

If we directly used the example value in the tests, an update in the library would break our integration tests and we could decide what to do: hold the change for a later release, or to cut a major release and document that breaking change so our users are prepared.

It's true that the usual way to proceed would be to just deprecate the Example constant with its original value and create a new semconv.RequestExample constant. But even if that's the usual process, I'd prefer to keep with the hardcoded names as a safety net.

[carrbs]

Hi @mariomac! Thank you for sharing this context with me, I totally understand your concern for the users.

I believe there's more flexibility here. We wouldn't need to be compliant with the latest stable specifications of OTEL, in fact we can specifically say which version of the semantic conventions are supported by beyla, i.e.:

import semconv "go.opentelemetry.io/otel/semconv/v1.24.0"

The semantic conventions in this directory are static and when new versions of the conventions are published, new definitions are generated (and added to a new directory) in the OTEL Go library.

So as an example, if semconv.Example == "example" in version 42, and we say "beyla supports the OTEL semantic conventions version 42", even if version 43 changes semconv.Example == "request.example", we don't need to change to version 43 until we're ready (e.g., a major release with other breaking changes).

Another thought: the way it is currently, (with hardcoded strings), doesn't let the users know which version of the semantic conventions are supported, which could also lead to confusion.

Let me know your thoughts! If you'd like to just have this PR be a small change to the string ( i.e. :s/otel.library.name/otel.scope.name), I can update it that way as well.

[mariomac]

Hi @carrbs ! Yeah, fixing the referred issue should be to just do that change, you can do it and we can merge it when we are ready to publish/report/document the breaking change.

Thank you very much for you collaboration!