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

Update telemetrygen span info #24286

Closed
edwintye opened this issue Jul 16, 2023 · 3 comments
Closed

Update telemetrygen span info #24286

edwintye opened this issue Jul 16, 2023 · 3 comments
Labels
cmd/telemetrygen telemetrygen command enhancement New feature or request

Comments

@edwintye
Copy link
Contributor

edwintye commented Jul 16, 2023

Component(s)

cmd/telemetrygen

Is your feature request related to a problem? Please describe.

Currently telemetrygen adds span.kind as an attribute L43 but "Span Kind" is neither a resource or an attribute and should be set separately via say trace.WithSpanKind(trace.SpanKindClient) instead.

Also for the purpose of testing it may be useful to have the ability to set the status.code.

Describe the solution you'd like

Update telemetrygen in worker.go so something like (at around L43)

		ctx, sp := tracer.Start(context.Background(), "lets-go", trace.WithAttributes(
 --                     attribute.String("span.kind", "client"), // is there a semantic convention for this?
			semconv.NetPeerIPKey.String(fakeIP),
			semconv.PeerServiceKey.String("telemetrygen-server"),
		),
++			trace.WithSpanKind(trace.SpanKindClient),
		)

and for the server

		_, child := tracer.Start(childCtx, "okey-dokey", trace.WithAttributes(
--                      attribute.String("span.kind", "server"),
			semconv.NetPeerIPKey.String(fakeIP),
			semconv.PeerServiceKey.String("telemetrygen-client"),
		),
++			trace.WithSpanKind(trace.SpanKindServer),
		)

Then maybe hard code the status code at L68

		opt := trace.WithTimestamp(time.Now().Add(fakeSpanDuration))
++		child.SetStatus(codes.Ok, "Success")
		child.End(opt)
++		sp.SetStatus(codes.Ok, "Success")
		sp.End(opt)

or some alternative such as allow user inject via command line argument.

Describe alternatives you've considered

No response

Additional context

This can be easily verified using a rudimentary config locally

receivers:
  otlp:
    protocols:
      grpc:
      http:
exporters:
  logging:
    verbosity: detailed
service:
  pipelines:
    traces/raw:
      receivers: [otlp]
      processors: []
      exporters: [logging]

and running telemetrygen against it via

telemetrygen traces --otlp-insecure --traces 1

to confirm that telemetrygen currently produces

esource SchemaURL: https://opentelemetry.io/schemas/1.4.0
Resource attributes:
     -> service.name: Str(telemetrygen)
ScopeSpans #0
ScopeSpans SchemaURL:
InstrumentationScope telemetrygen
Span #0
    Trace ID       : 7369a96e4f44cec645ad55e0637e136f
    Parent ID      : 70692ee1b35b1411
    ID             : 05a70278c3c4fa07
    Name           : okey-dokey
    Kind           : Internal
    Start time     : 2023-07-16 18:53:34.178838 +0000 UTC
    End time       : 2023-07-16 18:53:34.178962 +0000 UTC
    Status code    : Unset
    Status message :
Attributes:
     -> span.kind: Str(server)
     -> net.peer.ip: Str(1.2.3.4)
     -> peer.service: Str(telemetrygen-client)

which has Kind: Internal and span.kind: "server" without a status code set. This caused slight confusion when we were trying to test out some tail sampling policy locally.

@edwintye edwintye added enhancement New feature or request needs triage New item requiring triage labels Jul 16, 2023
@github-actions github-actions bot added the cmd/telemetrygen telemetrygen command label Jul 16, 2023
@github-actions
Copy link
Contributor

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@mx-psi
Copy link
Member

mx-psi commented Jul 17, 2023

Setting span kind with WithSpanKind makes sense to me. We can open a separate PR to discuss the status code setting.

jpkrohling pushed a commit that referenced this issue Jul 21, 2023
…fo (#24303)

**Description:** Remove the span attribute `span.kind` and instead set
the `Kind` (which is a top level span information) directly using
`trace.WithSpanKind`.

**Link to tracking Issue:** #24286 

**Testing:** Added a rudimentary test to ensure that `Kind` is not
`Internal` which is the default when not specified.

**Documentation:** None. This is probably the original intention.
@crobert-1
Copy link
Member

Hello @edwintye, was this issue resolved with #24303, or is there still some work required? If it's complete we can close the issue.

@crobert-1 crobert-1 removed the needs triage New item requiring triage label Sep 8, 2023
@edwintye edwintye closed this as completed Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd/telemetrygen telemetrygen command enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants