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

Implement peer.service and associated features #2982

Merged
merged 32 commits into from
Jul 28, 2023
Merged

Conversation

zarirhamza
Copy link
Contributor

@zarirhamza zarirhamza commented Jul 19, 2023

peer.service is now a tag that extracts its value from a subset of source tags that are unique per integration.

Needs explicit @delner approval

@zarirhamza zarirhamza requested review from a team and delner July 19, 2023 21:23
@github-actions github-actions bot added core Involves Datadog core libraries integrations Involves tracing integrations tracing labels Jul 19, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2023

Codecov Report

Merging #2982 (a5a717e) into master (d6fcf01) will increase coverage by 0.00%.
The diff coverage is 98.85%.

@@           Coverage Diff            @@
##           master    #2982    +/-   ##
========================================
  Coverage   98.09%   98.10%            
========================================
  Files        1305     1306     +1     
  Lines       72724    73244   +520     
  Branches     3360     3365     +5     
========================================
+ Hits        71336    71853   +517     
- Misses       1388     1391     +3     
Files Changed Coverage Δ
...atadog/tracing/contrib/active_record/events/sql.rb 97.61% <ø> (-0.16%) ⬇️
...ng/contrib/active_support/cache/instrumentation.rb 86.11% <ø> (+0.39%) ⬆️
lib/datadog/tracing/contrib/ethon/multi_patch.rb 97.91% <ø> (-0.09%) ⬇️
lib/datadog/tracing/contrib/grpc/formatting.rb 98.21% <0.00%> (-1.79%) ⬇️
lib/datadog/tracing/contrib/racecar/event.rb 97.43% <ø> (+2.31%) ⬆️
...dog/tracing/contrib/active_record/multi_db_spec.rb 93.60% <ø> (-0.57%) ⬇️
...tadog/tracing/contrib/active_record/tracer_spec.rb 97.59% <ø> (-0.14%) ⬇️
...c/datadog/tracing/contrib/ethon/easy_patch_spec.rb 100.00% <ø> (ø)
.../datadog/tracing/contrib/ethon/multi_patch_spec.rb 100.00% <ø> (ø)
spec/datadog/tracing/contrib/rails/cache_spec.rb 98.30% <ø> (-0.14%) ⬇️
... and 93 more

... and 1 file with indirect coverage changes

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

@zarirhamza zarirhamza added this to the 1.13.0 milestone Jul 20, 2023
Copy link
Member

@GustavoCaso GustavoCaso left a comment

Choose a reason for hiding this comment

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

I want to clarify that the requested changes are not about the SpanAttributeSchema module discussion 😄 on #2982 (comment)

The request changes is about the missing specs on spec/datadog/core/configuration/option_spec.rb and the addition of env_to_hash

lib/datadog/core/configuration/option.rb Show resolved Hide resolved
lib/datadog/core/environment/variable_helpers.rb Outdated Show resolved Hide resolved
Copy link
Contributor

@delner delner left a comment

Choose a reason for hiding this comment

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

@zarirhamza and I talked about our plans for this. We identified there's a little more work to do to make peer.service configurable. This should be in-flight; once its merged in to this branch, I think this will be ready.

@GustavoCaso
Copy link
Member

As far as the code, it is LGTM.

@zarirhamza, thanks for adding type :hash support to the core configuration options.

As for the peer.service logic, I can not review if it is complete or not 😄 since I don't know how it should behave. I will defer that to @delner and @zarirhamza

…tags to each integration that implements them
* Enables peer.service logic to always be v1
Implements `peer.service` in target integrations and removes old usages of `peer.service` while updating tests
Copy link
Contributor

@delner delner left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you for all your hard work @zarirhamza!

@TonyCTHsu TonyCTHsu merged commit 038eeeb into master Jul 28, 2023
204 checks passed
@TonyCTHsu TonyCTHsu deleted the feature/peer.service branch July 28, 2023 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Involves Datadog core libraries integrations Involves tracing integrations tracing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants