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

Do not allow default ErrorHandler to be set as the ErrorHandler #5135

Closed
MrAlias opened this issue Apr 2, 2024 · 0 comments · Fixed by #5137
Closed

Do not allow default ErrorHandler to be set as the ErrorHandler #5135

MrAlias opened this issue Apr 2, 2024 · 0 comments · Fixed by #5137
Assignees
Labels
bug Something isn't working
Milestone

Comments

@MrAlias
Copy link
Contributor

MrAlias commented Apr 2, 2024

When testing, it is a common need to change the default error handler. It follows that the original should be restored to not affect other code using that default behavior.

func Test(t *testing.T) {
	orig := otel.GetErrorHandler()
	var got error
	handler := otel.ErrorHandlerFunc(func(err error) { got = err })
	otel.SetErrorHandler(handler)
	t.Cleanup(func() { otel.SetErrorHandler(orig) })
	/* ... */
}

Doing this results in the original being set as its own dependent:

runtime: goroutine stack exceeds 1000000000-byte limit
runtime: sp=0xc0206e0388 stack=[0xc0206e0000, 0xc0406e0000]
fatal error: stack overflow

runtime stack:
runtime.throw({0x642212?, 0x1?})
	/usr/lib/go/src/runtime/panic.go:1023 +0x5c fp=0x782b1f3ffca0 sp=0x782b1f3ffc70 pc=0x43b8bc
runtime.newstack()
	/usr/lib/go/src/runtime/stack.go:1103 +0x5bd fp=0x782b1f3ffe50 sp=0x782b1f3ffca0 pc=0x45783d
runtime.morestack()
	/usr/lib/go/src/runtime/asm_amd64.s:616 +0x7a fp=0x782b1f3ffe58 sp=0x782b1f3ffe50 pc=0x472eda

goroutine 200 gp=0xc000102e00 m=9 mp=0xc0001b0008 [running]:
go.opentelemetry.io/otel/internal/global.(*ErrDelegator).Handle(0xc00011a090?, {0x6a9520?, 0xc000612520?})
	/home/tyler/go/src/go.opentelemetry.io/otel/internal/global/handler.go:36 +0x45 fp=0xc0206e0398 sp=0xc0206e0390 pc=0x595aa5
go.opentelemetry.io/otel/internal/global.(*ErrDelegator).Handle(0x0?, {0x6a9520?, 0xc000612520?})
	/home/tyler/go/src/go.opentelemetry.io/otel/internal/global/handler.go:37 +0x28 fp=0xc0206e03c0 sp=0xc0206e0398 pc=0x595a88
go.opentelemetry.io/otel/internal/global.(*ErrDelegator).Handle(0x0?, {0x6a9520?, 0xc000612520?})
	/home/tyler/go/src/go.opentelemetry.io/otel/internal/global/handler.go:37 +0x28 fp=0xc0206e03e8 sp=0xc0206e03c0 pc=0x595a88
go.opentelemetry.io/otel/internal/global.(*ErrDelegator).Handle(0x0?, {0x6a9520?, 0xc000612520?})
	/home/tyler/go/src/go.opentelemetry.io/otel/internal/global/handler.go:37 +0x28 fp=0xc0206e0410 sp=0xc0206e03e8 pc=0x595a88
go.opentelemetry.io/otel/internal/global.(*ErrDelegator).Handle(0x0?, {0x6a9520?, 0xc000612520?})
	/home/tyler/go/src/go.opentelemetry.io/otel/internal/global/handler.go:37 +0x28 fp=0xc0206e0438 sp=0xc0206e0410 pc=0x595a88
go.opentelemetry.io/otel/internal/global.(*ErrDelegator).Handle(0x0?, {0x6a9520?, 0xc000612520?})
	/home/tyler/go/src/go.opentelemetry.io/otel/internal/global/handler.go:37 +0x28 fp=0xc0206e0460 sp=0xc0206e0438 pc=0x595a88
go.opentelemetry.io/otel/internal/global.(*ErrDelegator).Handle(0x0?, {0x6a9520?, 0xc000612520?})
	/home/tyler/go/src/go.opentelemetry.io/otel/internal/global/handler.go:37 +0x28 fp=0xc0206e0488 sp=0xc0206e0460 pc=0x595a88
go.opentelemetry.io/otel/internal/global.(*ErrDelegator).Handle(0x0?, {0x6a9520?, 0xc000612520?})
[...]

Similar to the other global patterns, the error handler should check and ensure the default is not set as a dependent.

@MrAlias MrAlias added the bug Something isn't working label Apr 2, 2024
@MrAlias MrAlias self-assigned this Apr 3, 2024
MrAlias added a commit to MrAlias/opentelemetry-go that referenced this issue Apr 3, 2024
@github-project-automation github-project-automation bot moved this to Needs triage in Go: Triage Apr 3, 2024
@MrAlias MrAlias moved this from Needs triage to Low priority in Go: Triage Apr 3, 2024
@MrAlias MrAlias added this to the v1.25.0 milestone Apr 3, 2024
@github-project-automation github-project-automation bot moved this from Low priority to Closed in Go: Triage Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant