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

Span Id with all zeros cause index out of range #4573

Open
YakirOren opened this issue Jan 17, 2025 · 5 comments
Open

Span Id with all zeros cause index out of range #4573

YakirOren opened this issue Jan 17, 2025 · 5 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@YakirOren
Copy link

YakirOren commented Jan 17, 2025

Describe the bug

When Tempo queries for a traces that contains a span_id 0000000000000000
cause
panic serving 172.23.0.3:47754: runtime error: index out of range [7] with length 0

To Reproduce
Steps to reproduce the behavior:

  1. Start Tempo 2.7.0
  2. Push a trace to the /v1/traces endpoint a that contains a span with a span_id 0000000000000000
  3. Query the trace

Expected behavior
Tempo should return an error if this against the OTEL spec

Environment:

  • Infrastructure: laptop
  • Deployment tool: Docker

Additional Context

panic_output.log

tempo config

server:
  http_listen_port: 3200
  log_level: info

distributor:
  receivers:
    otlp:
      protocols:
        grpc:
          endpoint: "tempo:4317"
        http:
          endpoint: "tempo:4318"

storage:
  trace:
    backend: local 
    wal:
      path: /var/tempo/wal 
    local:
      path: /var/tempo/blocks
@joe-elliott
Copy link
Member

joe-elliott commented Jan 17, 2025

The panic starts here b/c the binary package assumes a slice length when you call the Uint64 conversion. It would be easy enough to patch this up, but I'm surprised nothing else is panic'ing with a span id of 0 length. I'm wondering if we should try to also fix this on ingestion.

Can you check compactors/ingesters/queriers to see if you're getting anything similar?

@YakirOren
Copy link
Author

In the logs I'm not seeing any panics on ingestion or compacting.

Calling the /api/traces/ and /api/v2/traces/ endpoints cause the panic.
the /api/search endpoint is ok

I think it would be best to fix it on ingestion
either return an error or accept it but generate a random span ID

@joe-elliott
Copy link
Member

joe-elliott commented Jan 21, 2025

Push a trace to the /v1/traces endpoint a that contains a span with a span_id 0000000000000000

As an aside are you sure the span you are pushing is all 0s? Is the id perhaps an empty slice?

I think we should do a quick fix to patch up the panic and make a longer term issue to handle an empty/all 0's span id on the write path. I'm not sure what the correct behavior there is. wdyt?

@joe-elliott joe-elliott added good first issue Good for newcomers help wanted Extra attention is needed labels Jan 21, 2025
@YakirOren
Copy link
Author

I've created this poc
tested on tempo:2.7.0

I'm not sure what the correct behavior there is

IMO all 0s spanID should be treated as invalid, and return an error

@joe-elliott
Copy link
Member

Tempo behavior is bound to a strange mix of common sense and the OTel spec. I cannot find anywhere in the spec that saying that an all 0s span id is invalid so I'm inclined to consume and store it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants