introduce internal/conventions package

[codeboten]

Description

The intention of this package is to simplify updating references to upstream semconv in the future. Today maybe components in this repository rely on the upstream semconv and every time the package updates, there are many changes required to follow it. My suggestion with this change is to provide an internal package that components can use instead to reeduce maintainer burden.

[TylerHelmuth]

What conventions would be exposed in this file? Only stable conventions or anything that Contrib needs?

[codeboten]

at this time, i'm proposing only exposing the values that contrib components require to reduce the cost of updating semconv deps

[codeboten]

im also proposing adding string comparisons for all values in the conventions file, this will make it easier for us to detect changes in future semconv

[TylerHelmuth]

A lot of contrib components depend on more semconvs than are listed here. When they upgrade, wont we still need to bump all those components?

[codeboten]

the update of the semconv package will only be needed in to internal/conventions package instead of all over the place

[codeboten]

right, this wont replace all imports of semconv, just the ones that can follow whatever the latest version is. As i'm updating imports, i'm also opening issues for components that are behind on semconv, to prompt owners to update the semconv

[TylerHelmuth]

Ok. If a component needs to use a new semconv and it doesn't exist in this package do they add it?

[TylerHelmuth]

ultimately I'm concerned we'll be trading dependency maintenance for module maintenance. This module will prevent big dependabot PRs with lots of go.mod changes, but will require keeping this package up to date with copies of the latest semantic conventions.

[codeboten]

i guess we're currently paying that cost anyways, but the semconv are implemented in many different components, which means we end up with various components supporting different semconvs, even the ones where they are using the same keys. For example, today if component A and component B are importing different versions of semconv in their code, the schema URLs for each one will be different, even though they may onlly use service.name in the code.

I think this makes it ultimately more difficult as a user to understand what is happening with any given release of the collector. But maybe that's not that big of a deal.

Today the updates of semconv is painful and maybe that's a one time cost once all components have updated to 1.37.0, but i've gone through the migration at least twice, and the process is pretty painful today.

[codeboten]

This module will prevent big dependabot PRs with lots of go.mod change

I'm not sure if this is something renovate does, but the imports need to be changed in the code itself, not the go.mod files

[dmathieu]

I'm a bit sceptical about this, as it could end up making upgrades harder, especially when a migration path may be required (see open-telemetry/opentelemetry-collector#14273).

The changes you mention to upgrade semconv are about changing the import path to include the new version?
If so, how about a script that would automate that?

[codeboten]

I'm a bit sceptical about this, as it could end up making upgrades harder

i'm not sure if it would. if a component is using a semconv that is changing, i would expect the following:

• internal/conventions updates to 1.X.X (incompatible version for some semconv)
• the tests in internal/conventions highlight the change and we can flag it at review
• the owner of the components that rely on that semconv can then know that a change is coming, and can import the old semconv in their package and at the same time provide the migration steps provided in the RFC

I guess I see this common code as a mechanism to make it easier for the maintainers of collector contrib to:

• update semconv for the components that only ever rely on stable semconv
• call out when changes in other collector components need to be made

With the current mechanism it's easy for components to fall behind, and components using the same keys to be out of sync with the semconvs they're using.

If so, how about a script that would automate that?

This could be scripted, but i'm not sure if the trade off of adding another script vs a common library is worth it.

It's also worth noting that at the moment, many components use the same semconv import in the code and the test code, which means that component owners may be blind sided if the name of the variable used doesn't change, but its value does. This could be addressed separately, by updating the individual tests to use strings instead.

[dmathieu]

if the name of the variable used doesn't change, but its value does.

Unless the semconv package changes how it generates packages, that's not possible.

[codeboten]

Unless the semconv package changes how it generates packages, that's not possible.

Right, but it does happen (see https://github.com/open-telemetry/semantic-conventions/blob/a43b2af59373f2119aeda0a328e6e3212349432e/schemas/1.38.0#L93)