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

otelhttp: Record metrics on timed out requests #4634

Closed
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

### Fixed

- Mimic the `context.WithoutCancel` behavior introduced in Go 1.21 to fix metric recording on timed out requests in `go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp`. (#4542)
carrbs marked this conversation as resolved.
Show resolved Hide resolved
- Fix `NewServerHandler` in `go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc` to correctly set the span status depending on the gRPC status. (#4587)
- Update `go.opentelemetry.io/contrib/detectors/aws/ecs` to fix the task ARN when it is not valid. (#3583)
- Do not panic in `go.opentelemetry.io/contrib/detectors/aws/ecs` when the container ARN is not valid. (#3583)
Expand Down
8 changes: 4 additions & 4 deletions instrumentation/net/http/otelhttp/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,33 +284,33 @@
})
}

func withoutCancel(ctx context.Context) context.Context {
if ctx.Done() == nil {
return ctx
func withoutCancel(parent context.Context) context.Context {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a note, including a link, about where this is copied from.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to do this, but it sounding like the better option might be the SDK native one (so long as we would not need the flexibility of calling those .Add and .Record methods in such a way where we want different behavior based on the context's cancelled state)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added, in case we opt to merge this PR. Thanks @MrAlias!

if parent == nil {
panic("cannot create context from nil parent")

Check warning on line 289 in instrumentation/net/http/otelhttp/handler.go

View check run for this annotation

Codecov / codecov/patch

instrumentation/net/http/otelhttp/handler.go#L289

Added line #L289 was not covered by tests
}
return withoutCancelCtx{ctx}
return withoutCancelCtx{parent}
}

type withoutCancelCtx struct {
c context.Context
}

func (withoutCancelCtx) Deadline() (deadline time.Time, ok bool) {
return

Check warning on line 299 in instrumentation/net/http/otelhttp/handler.go

View check run for this annotation

Codecov / codecov/patch

instrumentation/net/http/otelhttp/handler.go#L298-L299

Added lines #L298 - L299 were not covered by tests
}

func (withoutCancelCtx) Done() <-chan struct{} {
return nil

Check warning on line 303 in instrumentation/net/http/otelhttp/handler.go

View check run for this annotation

Codecov / codecov/patch

instrumentation/net/http/otelhttp/handler.go#L302-L303

Added lines #L302 - L303 were not covered by tests
}

func (withoutCancelCtx) Err() error {
return nil
}

func (w withoutCancelCtx) Value(key any) any {
return w.c.Value(key)

Check warning on line 311 in instrumentation/net/http/otelhttp/handler.go

View check run for this annotation

Codecov / codecov/patch

instrumentation/net/http/otelhttp/handler.go#L310-L311

Added lines #L310 - L311 were not covered by tests
}

func (w withoutCancelCtx) String() string {
return "withoutCancel"

Check warning on line 315 in instrumentation/net/http/otelhttp/handler.go

View check run for this annotation

Codecov / codecov/patch

instrumentation/net/http/otelhttp/handler.go#L314-L315

Added lines #L314 - L315 were not covered by tests
}