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

ActiveModelSerializer instrumentation does not make the span current #992

Open
renchap opened this issue May 17, 2024 · 6 comments · May be fixed by #1075
Open

ActiveModelSerializer instrumentation does not make the span current #992

renchap opened this issue May 17, 2024 · 6 comments · May be fixed by #1075
Labels
bug Something isn't working keep Ensures stale-bot keeps this issue/PR open

Comments

@renchap
Copy link

renchap commented May 17, 2024

Description of the bug

Spans occuring during a render call using AM:S are not set as children span of the AM:S render span

image

Here, the spans below the serializer are made by the serializer code.

Share details about your runtime

Operating system details: Linux, Ubuntu 20.04 LTS
RUBY_ENGINE: "ruby"
RUBY_VERSION: "3.2.2"
RUBY_DESCRIPTION: "ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin22]"

Share a simplified reproduction if possible

No reproduction, but the issue is that the event handler is setting itself as the current span here: https://github.com/open-telemetry/opentelemetry-ruby-contrib/blob/main/instrumentation/active_model_serializers/lib/opentelemetry/instrumentation/active_model_serializers/event_handler.rb#L14-L20

Pinging @robbkidd as he helped me on this

@renchap renchap added the bug Something isn't working label May 17, 2024
@robbkidd
Copy link
Member

robbkidd commented May 17, 2024

Thanks for opening this, @renchap!

My hypothesis is that ActiveModelSerializers instrumentation is using an older style of AS::Notification event subscription and handling. It does not start the span in a way that makes the serialization span the current span while it is doing work. (And it should.)

Copy link
Contributor

👋 This issue has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this issue will be closed eventually by the stale bot.

@github-actions github-actions bot added the stale Marks an issue/PR stale label Jun 17, 2024
@renchap
Copy link
Author

renchap commented Jun 17, 2024

Still a valid issue!

@github-actions github-actions bot removed the stale Marks an issue/PR stale label Jun 18, 2024
Copy link
Contributor

👋 This issue has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this issue will be closed eventually by the stale bot.

@github-actions github-actions bot added the stale Marks an issue/PR stale label Jul 18, 2024
@renchap
Copy link
Author

renchap commented Jul 19, 2024

Still valid!

@robbkidd robbkidd linked a pull request Jul 19, 2024 that will close this issue
1 task
@github-actions github-actions bot removed the stale Marks an issue/PR stale label Jul 20, 2024
Copy link
Contributor

👋 This issue has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this issue will be closed eventually by the stale bot.

@github-actions github-actions bot added the stale Marks an issue/PR stale label Aug 19, 2024
@kaylareopelle kaylareopelle added keep Ensures stale-bot keeps this issue/PR open and removed stale Marks an issue/PR stale labels Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working keep Ensures stale-bot keeps this issue/PR open
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants