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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass the service name to instantiation spans #3383

Closed

Conversation

benoittgt
Copy link

@benoittgt benoittgt commented Jan 15, 2024

What does this PR do?

This PR fix #3359

This is a draft because I think we need to have a discussion first on the issue.

Motivation:

Having the same service name for "instantiation spans" and "sql spans".

Additional Notes:

I don't know how this change can impact end user.

How to test the change?

It has been tested in spec suite but should be tested E2E. 馃槃

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.

Unsure? Have a question? Request a review!

@benoittgt benoittgt force-pushed the service-name-on-instantiation branch from 2c2cdb8 to e5f064a Compare January 15, 2024 23:36
@@ -110,7 +109,7 @@

expect(instantiation_span.name).to eq('active_record.instantiation')
expect(instantiation_span.span_type).to eq('custom')
expect(instantiation_span.service).to eq(tracer.default_service) # Because within parent
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could test also the fallback when

service_name: database_service

is not defined.

@TonyCTHsu
Copy link
Contributor

I left a comment on the original issue.
#3359 (comment)

@benoittgt
Copy link
Author

benoittgt commented Jan 18, 2024

Thanks @TonyCTHsu
Closing because of #3359 (comment)

@benoittgt benoittgt closed this Jan 18, 2024
@benoittgt benoittgt deleted the service-name-on-instantiation branch January 21, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrations Involves tracing integrations tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ActiveRecord SQL spans don't have the same service name as Instantiation spans
2 participants