Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deps: support OTel tracing v1.25.0 #191

Merged
merged 2 commits into from
Apr 24, 2024
Merged

Conversation

inge4pres
Copy link
Contributor

Reason for this PR

Addresses #189.

Details

Minor version upgrades to the OTel suites and implement the interface in DummySpan.

go.mod Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
@Baliedge
Copy link
Collaborator

@inge4pres I've recently merged #194, which addresses updating Golang version to 1.21. Please update from master branch. Then we can merge this.

Thanks for your contribution.

"github.com/sirupsen/logrus"
"github.com/uptrace/opentelemetry-go-extra/otellogrus"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Should revert this unnecessary change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah thanks, that's my IDE formatting imports, will do

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. It placed organizational imports in a 3rd section. That's a new thing with Golang. So it's not a mistake.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but there are 4 sections now.

@inge4pres
Copy link
Contributor Author

Thanks for your contribution.

Thank you for the amazing project given to the community!

I rebased on most recent HEAD

"github.com/sirupsen/logrus"
"github.com/uptrace/opentelemetry-go-extra/otellogrus"

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. It placed organizational imports in a 3rd section. That's a new thing with Golang. So it's not a mistake.

Copy link
Collaborator

@Baliedge Baliedge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is failing due to a required change that goes with the OTel update. Apply this patch:

diff --git a/tracing/tracing.go b/tracing/tracing.go
index 7306a70..9831fb4 100644
--- a/tracing/tracing.go
+++ b/tracing/tracing.go
@@ -15,7 +15,7 @@ import (
        "go.opentelemetry.io/otel/propagation"
        "go.opentelemetry.io/otel/sdk/resource"
        sdktrace "go.opentelemetry.io/otel/sdk/trace"
-       semconv "go.opentelemetry.io/otel/semconv/v1.21.0"
+       semconv "go.opentelemetry.io/otel/semconv/v1.24.0"
        "go.opentelemetry.io/otel/trace"

        "github.com/mailgun/holster/v4/errors"

Signed-off-by: inge4pres <[email protected]>
@inge4pres
Copy link
Contributor Author

Apply this patch:

Thanks! 🙏🏼 just pushed

Copy link
Collaborator

@Baliedge Baliedge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

@Baliedge Baliedge merged commit ebe61f9 into mailgun:master Apr 24, 2024
3 checks passed
@inge4pres
Copy link
Contributor Author

Thanks for the guidance @Baliedge 🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants