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.

Sign up for GitHub

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

Jump to bottom

span.IsRecording() returns true even when Sampler indicates it should be DROPPED #1664

Closed
evantorrie opened this issue Mar 5, 2021 · 1 comment · Fixed by #1750
Closed

span.IsRecording() returns true even when Sampler indicates it should be DROPPED #1664

evantorrie opened this issue Mar 5, 2021 · 1 comment · Fixed by #1750
Assignees
@bryan-aguilar
Labels
area:trace Part of OpenTelemetry tracing bug Something isn't working good first issue Good for newcomers pkg:SDK Related to an SDK package
Projects
OpenTelemetry Go RC
Milestone
RC1

Comments

@evantorrie
Copy link
Contributor

Currently the Go code for span.IsRecording() returns true if s.endTime.IsZero() == true.

But the default SDK's span creation function will return a span with span.endTime.IsZero() (default initialized) even if the associated Sampler.ShouldSample() function returns DROP.

Based on the current specification, DROP should result in IsRecording() == false

DROP - IsRecording() == false, span will not be recorded and all events and attributes will be dropped.

Possibly related issue in opentelemetry-specification: open-telemetry/opentelemetry-specification#1507
@MrAlias MrAlias added area:trace Part of OpenTelemetry tracing bug Something isn't working pkg:SDK Related to an SDK package release:required-for-ga labels Mar 5, 2021
@MrAlias MrAlias added this to To do in OpenTelemetry Go RC via automation Mar 5, 2021
@MrAlias MrAlias added this to the RC1 milestone Mar 5, 2021
@MrAlias MrAlias mentioned this issue Mar 9, 2021
Closed
2 tasks
@Aneurysm9 Aneurysm9 added the good first issue Good for newcomers label Mar 24, 2021
@bryan-aguilar
Copy link
Contributor

@Aneurysm9 I'd like to take this one on. I'll dig into the spec a bit to make sure everything is clear before starting.

This was referenced Mar 26, 2021
Closed
Merged
@MrAlias MrAlias moved this from To do to In progress in OpenTelemetry Go RC Mar 29, 2021
OpenTelemetry Go RC automation moved this from In progress to Done Mar 30, 2021
@bryan-aguilar bryan-aguilar mentioned this issue Apr 12, 2021
Closed
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bryan-aguilar bryan-aguilar

Labels
area:trace Part of OpenTelemetry tracing bug Something isn't working good first issue Good for newcomers pkg:SDK Related to an SDK package
Projects
No open projects
OpenTelemetry Go RC
  
Done
Milestone
RC1
4 participants
@Aneurysm9 @MrAlias @evantorrie @bryan-aguilar