Skip to content

Conversation

@clux
Copy link
Member

@clux clux commented Mar 6, 2023

to avoid further confusion for stuff like #44. sets up a minimal context to verify that we get a trace.
saved under a shortcut for just test-telemetry that requires something like just forward-tempo to work.

can be made fully automated (see below), but it requires a valid tempo instance on CI or some mocking setup for tracers that does not produce invalid traceids (afaikt the mock tracers all produce zeroes).

Signed-off-by: clux <sszynrae@gmail.com>
@clux
Copy link
Member Author

clux commented Mar 6, 2023

hm. i think the bug is actually invalid; it "works for me" when using just run-telemetry having port-forwarded to a local tempo instance.

@clux
Copy link
Member Author

clux commented Mar 6, 2023

doesn't look like i can test this locally without a full tracer. even their noop tracers return zeroes:

impl PreSampledTracer for noop::NoopTracer {
    fn sampled_context(&self, data: &mut crate::OtelData) -> OtelContext {
        data.parent_cx.clone()
    }

    fn new_trace_id(&self) -> otel::TraceId {
        otel::TraceId::INVALID
    }

    fn new_span_id(&self) -> otel::SpanId {
        otel::SpanId::INVALID
    }
}

Signed-off-by: clux <sszynrae@gmail.com>
@clux
Copy link
Member Author

clux commented Mar 6, 2023

ugh, i'd have to basically install tempo into the cluster for the integration tests using the chart with some very simple values and helm using something like:

      - uses: azure/setup-helm@v3
      - run: helm install grafana/tempo --wait -f values.yaml
      - run: just forward-tempo
      - run: just test-telemetry

with chart values.yaml as something like:

replicas: 1
tempo:
  retention: 24h
  authEnabled: false
  server:
    http_listen_port: 3100
  storage:
    trace:
      backend: local
      local:
        path: /var/tempo/traces
      wal:
        path: /var/tempo/wal
  receivers:
    otlp:
      protocols:
        http:
          endpoint: "0.0.0.0:55681"
        grpc:
          endpoint: "0.0.0.0:4317"
    jaeger:
      protocols:
        grpc:
          endpoint: "0.0.0.0:14250"
persistence:
  enabled: false

(this works in my old tempo pin at 1.0.1, but probably needs changes)

i'll leave this to a separate PR and leave the test as manual for now.

@clux clux marked this pull request as ready for review March 6, 2023 07:58
@clux clux changed the title TraceId WIP Add manual tests for valid TraceIds Mar 6, 2023
Signed-off-by: clux <sszynrae@gmail.com>
@clux clux linked an issue Mar 6, 2023 that may be closed by this pull request
Signed-off-by: clux <sszynrae@gmail.com>
@clux clux merged commit b31fb25 into main Mar 6, 2023
@clux clux deleted the telemetry-traceid branch March 6, 2023 08:09
@clux clux mentioned this pull request Mar 6, 2023
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.

trace and span IDs are invalid (zero)

2 participants