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

[BUG] Fatal issue in sqlcomment.go #2837

Open
fdc-michael-grosshans opened this issue Aug 30, 2024 · 2 comments
Open

[BUG] Fatal issue in sqlcomment.go #2837

fdc-michael-grosshans opened this issue Aug 30, 2024 · 2 comments
Assignees
Labels
bug unintended behavior that has to be fixed

Comments

@fdc-michael-grosshans
Copy link

fdc-michael-grosshans commented Aug 30, 2024

There is a mismatch in SQLCommentCarrier & textmap propagator about the fact that a span in spanContext can be nil, which leads to panics in code.

  1. In sqlcomment.go the SQLCommentCarrier assumes that a spanContext always has a non-nil span,
func (c *spanContext) meta(key string) (val string, ok bool) {
        // will panic if c.span is nil
	c.span.RLock()
	defer c.span.RUnlock()
	val, ok = c.span.Meta[key]
	return val, ok
}

func (c *SQLCommentCarrier) Inject(spanCtx ddtrace.SpanContext) error {
        .....
	case DBMPropagationModeService:
		if ctx, ok := spanCtx.(*spanContext); ok {
                        // calling meta assumes ctx.span to me non-nil
			if e, ok := ctx.meta(ext.Environment); ok && e != "" {
				tags[sqlCommentEnv] = e
			}
  1. In textmap.go the extractor creates a spanContext without an actual span.

Version
latest

Describe what happened:
A spanContext extracted from a textmap propagator that is injected into an SQLCommentCarrier will lead to panic.

Describe what you expected:
Definetly no panic in a tracing library.

Steps to reproduce the issue:

  • Extract the context using textmap.go
  • Inject via sqlcomment.go

Additional environment details (Version of Go, Operating System, etc.):

@fdc-michael-grosshans fdc-michael-grosshans added the bug unintended behavior that has to be fixed label Aug 30, 2024
@github-actions github-actions bot added the needs-triage New issues that have not yet been triaged label Aug 30, 2024
@darccio
Copy link
Member

darccio commented Aug 30, 2024

@fdc-michael-grosshans Thanks for reaching out. Please, can you confirm your environment details? Specially dd-trace-go and Go versions.

@darccio darccio removed the needs-triage New issues that have not yet been triaged label Aug 30, 2024
@fdc-michael-grosshans
Copy link
Author

we use dd-trace-go 1.66 (but it is still existing in the newest version, from reading the code.)
and we use go1.22.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug unintended behavior that has to be fixed
Projects
None yet
Development

No branches or pull requests

3 participants