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

Fix service_nam typo to service_name #2296

Merged
merged 1 commit into from
Sep 28, 2022
Merged

Fix service_nam typo to service_name #2296

merged 1 commit into from
Sep 28, 2022

Conversation

jackweinbender
Copy link
Contributor

Just a simple typo fix in the configuration file of the Graphql integration. I don't know if this is the source of some of my recent issues, but if the option is going to be there... it should be right.

@jackweinbender jackweinbender requested a review from a team September 28, 2022 18:03
@codecov-commenter
Copy link

Codecov Report

Merging #2296 (06ef469) into master (bea1360) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2296      +/-   ##
==========================================
- Coverage   97.58%   97.57%   -0.02%     
==========================================
  Files        1094     1094              
  Lines       57394    57392       -2     
==========================================
- Hits        56010    55999      -11     
- Misses       1384     1393       +9     
Impacted Files Coverage Δ
.../tracing/contrib/graphql/configuration/settings.rb 100.00% <100.00%> (ø)
...ontrib/sidekiq/server_internal_tracer/heartbeat.rb 36.00% <0.00%> (-28.00%) ⬇️
lib/datadog/core/workers/polling.rb 96.55% <0.00%> (-3.45%) ⬇️
lib/datadog/core/diagnostics/environment_logger.rb 98.41% <0.00%> (-1.59%) ⬇️
...adog/tracing/contrib/sidekiq/client_tracer_spec.rb 100.00% <0.00%> (+2.08%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@marcotc marcotc left a comment

Choose a reason for hiding this comment

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

Thank you, @jackweinbender!

Funny enough, because Datadog::Tracing::Contrib::GraphQL::Configuration::Settings inherits from Datadog::Tracing::Contrib::Configuration::Settings, which contains:

The service_name setting is actually working as expected, despite the typo:

require 'ddtrace'
Datadog::Tracing::Contrib::GraphQL::Configuration::Settings.instance_methods.select{|x|x.start_with?('service_name')}
# => [:service_name=, :service_name]

Your changes are still welcome, as we'd like to have precise tracking of which integrations explicit want a service_name configuration vs integrations that only have it because it's the default behavior.

@marcotc marcotc added the bug Involves a bug label Sep 28, 2022
@marcotc marcotc merged commit c439520 into DataDog:master Sep 28, 2022
@github-actions github-actions bot added this to the 1.5.0 milestone Sep 28, 2022
@lloeki
Copy link
Contributor

lloeki commented Sep 29, 2022

Hey @jackweinbender, this change was just released in v1.5.0!

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

Successfully merging this pull request may close these issues.

None yet

4 participants