Skip to content

Feature/pass opts to with span#137

Open
jeffdeville wants to merge 7 commits intomarcdel:mainfrom
withbelay:feature/pass-opts-to-with-span
Open

Feature/pass opts to with span#137
jeffdeville wants to merge 7 commits intomarcdel:mainfrom
withbelay:feature/pass-opts-to-with-span

Conversation

@jeffdeville
Copy link
Copy Markdown

@jeffdeville jeffdeville commented Nov 6, 2023

This does 2 things:

  1. There's a race condition in the async tests where the test pid is being overwritten on each setup block. (OtelHelper.otel_pid_reporter is constantly reconfiguring telemetry). Turning async off, and stopping opentelemetry when the tests start resolved the issue
  2. We need to be able to set the span.kind attribute (producer/consumer/client/server/internal) in order to get service graphs to be configured in grafana. So we've added kind as an option, and validated the value.
  3. We also wanted the ability to set arbitrary span.attributes, so we allowed attributes to be passed in via opts.
    Note If the input parameters tried to set a span attribute that was also set via the attributes key, the explicitly set values in attributes take precedence.

jeffdeville and others added 7 commits November 6, 2023 17:01
Creating/Destroying the otel_exporter_pid for each test was not cooperating with all the tests running simultaneously. Almost certainly due to the race condition in OtelHlper.otel_pid_reporter altering the global state to configure where the spans were being sent

Co-authored-by: Gray <surrsurus@users.noreply.github.com>
Did this because the first test was always failing

Co-authored-by: Gray <surrsurus@users.noreply.github.com>
@marcdel
Copy link
Copy Markdown
Owner

marcdel commented Jan 5, 2024

Hey @jeffdeville,

I'm just getting around to these PRs. Thanks for the work you've done here!

Could you give me an example of when you'd want arbitrary static attributes? I'm trying to weigh the added complexity of additional option against calling Span.set_attributes or Attributes.set in the body of the function.

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.

2 participants