OtelFiber: Use tracetest

[emanuelef]

SA1019: "go.opentelemetry.io/otel/oteltest" is deprecated: This package contains an alternate implementation of the OpenTelemetry SDK. This means it will diverge from the one commonly used in real world operations and therefore will not provide adequate testing guarantees for users. Because of this, this package should not be used to test performance or behavior of code running OpenTelemetry. Instead, the SpanRecorder from the go.opentelemetry.io/otel/sdk/trace/tracetest package can be registered with the default SDK (go.opentelemetry.io/otel/sdk/trace) as a SpanProcessor and used to test. This will ensure code will work with the default SDK. If users do not want to include a dependency on the default SDK it is recommended to run integration tests in their own module to isolate the dependency (see go.opentelemetry.io/otel/bridge/opencensus/test as an example).

https://pkg.go.dev/go.opentelemetry.io/otel/oteltest

This is the approach to use if moving to go.opentelemetry.io/otel/sdk/trace/tracetest
https://github.com/open-telemetry/opentelemetry-go/tree/main/bridge/opencensus/test

[gaby]

@emanuelef Can you rebase, and remove 1.18.x from the otelfiber test suite

[emanuelef]

@emanuelef Can you rebase, and remove 1.18.x from the otelfiber test suite

Sure, but will that mean 1.18 is no longer supported in all the tests ?

[gaby]

@emanuelef Can you rebase, and remove 1.18.x from the otelfiber test suite

Sure, but will that mean 1.18 is no longer supported in all the tests ?

Only remove it for otelfiber. :-) But yes, it means we will have to release a major release for this middleware to drop support for go 1.18.x

[gaby]

@emanuelef Can you rebase, and remove 1.18.x from the otelfiber test suite

Sure, but will that mean 1.18 is no longer supported in all the tests ?

Only remove it for otelfiber. :-) But yes, it means we will have to release a major release for this middleware to drop support for go 1.18.x

Oh, I see what you mean... The tests CI is running all the middleware's in one place.

[ReneWerner87]

pls increate the major number in the go.mod version

https://github.com/gofiber/contrib/blob/main/otelfiber/go.mod#L1
to

module github.com/gofiber/contrib/otelfiber/v2
and the readme instructions

[emanuelef]

@emanuelef Can you rebase, and remove 1.18.x from the otelfiber test suite

Sure, but will that mean 1.18 is no longer supported in all the tests ?

Only remove it for otelfiber. :-) But yes, it means we will have to release a major release for this middleware to drop support for go 1.18.x

Oh, I see what you mean... The tests CI is running all the middleware's in one place.

Indeed, changing that will drop 1.18 testing for all the others. Should we split the test action ? But I guess in case that would be another PR.

[gaby]

@emanuelef Can you rebase, and remove 1.18.x from the otelfiber test suite

Sure, but will that mean 1.18 is no longer supported in all the tests ?

Only remove it for otelfiber. :-) But yes, it means we will have to release a major release for this middleware to drop support for go 1.18.x

Oh, I see what you mean... The tests CI is running all the middleware's in one place.

Indeed, changing that will drop 1.18 testing for all the others. Should we split the test action ? But I guess in case that would be another PR.

I'm thinking whats the best solution for this. In the storage repo every module has a separate file. We may need to do that for contrib

[ReneWerner87]

pls increate the major number in the go.mod version

https://github.com/gofiber/contrib/blob/main/otelfiber/go.mod#L1 to

module github.com/gofiber/contrib/otelfiber/v2 and the readme instructions

@emanuelef can you resolve the conflicts with the master and add the "v2" prefix in the go.mod file

like here
gofiber/storage@ec921b8

[ReneWerner87]

don´t forget the readme.md

[emanuelef]

Some questions:

• will there be a changelog to explain why v2 was needed ?
• how are we going to handle tests with different go versions for each module ?
• do we need to add a coverage badge , so we know what percentage of the code is covered by the test ?
• Is it ok to expose the metric constants ?

[ReneWerner87]

will there be a changelog to explain why v2 was needed ?

yes the release notes for the tag

how are we going to handle tests with different go versions for each module ?

each test workflow has its own defined golang versions, so should not be a problem

do we need to add a coverage badge , so we know what percentage of the code is covered by the test ?

if that is possible and you can do that, gladly

Is it ok to expose the metric constants ?

is it conceivable that these are used outside? Then yes, otherwise I would not expose them

[emanuelef]

so I guess the only reason for v2 is that minimum Go version is now 1.19 ?
Also I wasn't sure why that was needed.

[emanuelef]

will there be a changelog to explain why v2 was needed ?

yes the release notes for the tag

how are we going to handle tests with different go versions for each module ?

each test workflow has its own defined golang versions, so should not be a problem

@gaby can you comment on that ? I can see the GH action is the same for all the other middlewares.

do we need to add a coverage badge , so we know what percentage of the code is covered by the test ?

if that is possible and you can do that, gladly

Is it ok to expose the metric constants ?

is it conceivable that these are used outside? Then yes, otherwise I would not expose them

[ReneWerner87]

will there be a changelog to explain why v2 was needed ?

yes the release notes for the tag

how are we going to handle tests with different go versions for each module ?

each test workflow has its own defined golang versions, so should not be a problem

@gaby can you comment on that ? I can see the GH action is the same for all the other middlewares.

do we need to add a coverage badge , so we know what percentage of the code is covered by the test ?

if that is possible and you can do that, gladly

Is it ok to expose the metric constants ?

is it conceivable that these are used outside? Then yes, otherwise I would not expose them

good point, we need to split the test workflows like in the storage repository
https://github.com/gofiber/storage/tree/main/.github/workflows
thought that was already the case

otherwise we would customize the golang version for all contrib packages, but we don't want to do that

[gaby]

@emanuelef @ReneWerner87 I can do a separate PR to separate all the tests, then we can update this PR?

[ReneWerner87]

ok

[gaby]

@emanuelef Test changes have been merged, you can rebase now :-)

[emanuelef]

@gaby I rebased and removed 1.18 from test-otelfiber.yml, but I can see 1.18 test still triggered. I might be missing something.

[emanuelef]

ok, I've just had to delete test.yml in my PR.

[emanuelef]

If users do not want to include a dependency on the default SDK it is recommended to run integration tests in their own module to isolate the dependency (see go.opentelemetry.io/otel/bridge/opencensus/test as an example).

Before closing, do we want to create a module just for the test to avoid including the tracetest dependency ?

[emanuelef]

@ReneWerner87 is this PR not going to be merged because of the breaking changes ?

[ReneWerner87]

sorry had been waiting for the changes for the test workflow and module and then lost focus for this pull request