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

azuremonitorexporter assign wrong parentId to span events #27233

Closed
Obi-Dann opened this issue Sep 27, 2023 · 8 comments
Closed

azuremonitorexporter assign wrong parentId to span events #27233

Obi-Dann opened this issue Sep 27, 2023 · 8 comments
Labels

Comments

@Obi-Dann
Copy link

Component(s)

exporter/azuremonitor

What happened?

Description

azuremonitorexporter seems to assign wrong parentId to span events. As the results, span events have the same parentId as the span, but should have spanId as the parentId IIUC.

image
image

It looks like the issue is here
newEnvelope implementation should be different for span events, setting parentId to the spanId instead of always setting parentId to the span's parent id

Steps to Reproduce

Configure a collector with azuremonitor as an exported, set spaneventsenabled: true and ensure to send spans with events to the collector.

Expected Result

Span events should have the spanId set as their parentId

Actual Result

Span events have the span's parentId set as their parentId

Collector version

0.85.0

Environment information

Environment

OS: macOS 13.6

OpenTelemetry Collector configuration

receivers:
  otlp:
    protocols:
      http:

exporters:
  logging:
  azuremonitor:
    instrumentation_key: ""
    spaneventsenabled: true

processors:
  batch:
    timeout: 10s
    send_batch_size: 1000

service:
  pipelines:
    traces:
      receivers: [otlp]
      processors: []
      exporters: [logging, azuremonitor]
    logs:
      receivers: [otlp]
      processors: []
      exporters: [logging, azuremonitor]

Log output

No response

Additional context

No response

@Obi-Dann Obi-Dann added bug Something isn't working needs triage New item requiring triage labels Sep 27, 2023
@github-actions
Copy link
Contributor

Pinging code owners:

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

Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

  • exporter/azuremonitor: @pcwiese
  • needs: Github issue template generation code needs this to generate the corresponding labels.

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

@crobert-1
Copy link
Member

The semantic conventions are a bit unclear here as there's no default attribute in a span event for parentID. I think your interpretation is fair in that it makes sense for the events of a span to be considered the children of the span itself. Wording it another way, the span can be considered a parent of all of its own events.

@pcwiese may have different thoughts as code owner though, and this may also break some people's existing assumptions and queries in the backend.

Relevant documentation: Semantic conventions

@crobert-1 crobert-1 removed Stale needs triage New item requiring triage labels Dec 7, 2023
@alaendle
Copy link
Contributor

alaendle commented Jan 17, 2024

We also encountered this issue while forwarding Zipkin events to Azure Monitor.
The current implementation just creates wrong impressions - the traces are one level-up in the tree; so one could expect that the trace was generated by the producer - even if the consumer produces the trace.
See the following AppInsights screenshot (I've added the red circles/arrows to illustrate the problem):
grafik
Please let me know if I could provide further information.

Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

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

@github-actions github-actions bot added the Stale label Mar 18, 2024
@alaendle
Copy link
Contributor

This is solved as soon as #30659 is merged - so if anyone with rights to merge this PR reads this, please make progress at this point 📯 .

@github-actions github-actions bot removed the Stale label Mar 19, 2024
TylerHelmuth pushed a commit that referenced this issue Apr 18, 2024
…30659)

**Description:** Corrected parent for exported span events.
For details please take a look at the corresponding issue.
I think the change is pretty straight forward - one line added.
However I'm not a go-expert - so there might exist a more elegant
implementation.

**Link to tracking Issue:** #27233

**Testing:** Adapted existing units test.

**Documentation:** n/a

(fyi @pcwiese)
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this issue May 8, 2024
…pen-telemetry#30659)

**Description:** Corrected parent for exported span events.
For details please take a look at the corresponding issue.
I think the change is pretty straight forward - one line added.
However I'm not a go-expert - so there might exist a more elegant
implementation.

**Link to tracking Issue:** open-telemetry#27233

**Testing:** Adapted existing units test.

**Documentation:** n/a

(fyi @pcwiese)
Copy link
Contributor

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

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

@github-actions github-actions bot added the Stale label May 20, 2024
@crobert-1
Copy link
Member

Resolved by #30659

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants