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

feat!: Set a default service name (DD_SERVICE) when no service is configured #158

Merged
merged 4 commits into from
Oct 8, 2024

Conversation

zacharycmontoya
Copy link
Contributor

@zacharycmontoya zacharycmontoya commented Oct 3, 2024

Description

Adds a default service for the configuration if none was set in the DD_SERVICE environment variable or user configuration. This is a breaking change as previously a missing service would throw a configuration initialization error.

The default value will be set to the name of the executable.

Motivation

This aligns the cpp tracer behavior for an unconfigured DD_SERVICE with the expected behavior of all the tracers in our Config Consistency initiative.

Additional Notes

Also updates the parametric test so that the DD_ENV behavior works as expected.

This has been tested on Ubuntu 24.04 (run on a Windows machine in WSL), Windows 11, and macOS Monterey

Jira ticket: APMAPI-709 APMAPI-708

@zacharycmontoya zacharycmontoya changed the title feat!: Implement Config Consistency for DD_SERVICE when no service is configured feat!: Set a default service name (DD_SERVICE) when no service is configured Oct 3, 2024
@pr-commenter
Copy link

pr-commenter bot commented Oct 3, 2024

Benchmarks

Benchmark execution time: 2024-10-07 14:09:05

Comparing candidate commit bb5065c in PR branch zach.montoya/config-consistency/dd-service with baseline commit 85370e7 in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.

@zacharycmontoya zacharycmontoya force-pushed the zach.montoya/config-consistency/dd-service branch from 4bd244f to 51d33e8 Compare October 3, 2024 23:34
…ame value when none is provided by DD_SERVICE or configuration

Issues: APMAPI-709
…figuration so we can test default behaviors

Issues: APMAPI-708 , APMAPI-709
@zacharycmontoya zacharycmontoya force-pushed the zach.montoya/config-consistency/dd-service branch from 51d33e8 to c312c99 Compare October 3, 2024 23:36
@zacharycmontoya zacharycmontoya marked this pull request as ready for review October 3, 2024 23:41
@zacharycmontoya zacharycmontoya requested a review from a team as a code owner October 3, 2024 23:41
@zacharycmontoya zacharycmontoya requested review from Anilm3 and removed request for a team October 3, 2024 23:41
@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.66%. Comparing base (85370e7) to head (bb5065c).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #158   +/-   ##
=======================================
  Coverage   94.66%   94.66%           
=======================================
  Files          72       72           
  Lines        3804     3806    +2     
=======================================
+ Hits         3601     3603    +2     
  Misses        203      203           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@dmehala dmehala left a comment

Choose a reason for hiding this comment

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

Propose changes to consider cases where the underlying function can fail for Darwin and Windows NT.

src/datadog/platform_util.cpp Outdated Show resolved Hide resolved
src/datadog/platform_util.cpp Outdated Show resolved Hide resolved
@zacharycmontoya zacharycmontoya merged commit 89426d5 into main Oct 8, 2024
22 checks passed
@zacharycmontoya zacharycmontoya deleted the zach.montoya/config-consistency/dd-service branch October 8, 2024 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants