Skip to content

Commit

Permalink
chore(crashtracking): enable wait_for_receiver field to be configur…
Browse files Browse the repository at this point in the history
…ed via env var (#10233)

### Change Overview
This PR enables the `wait_for_receiver` field to be set via
`DD_CRASHTRACKING_WAIT_FOR_RECEIVER` environment variable. If unset, the
default is `true`.

### Motivation
[System-tests for
crashtracking](DataDog/system-tests#2884)
exposed that the Python crashtracker needs the `wait_for_receiver` field
to be set to `true` to capture crashes and reliably send telemetry
events to the agent. While set to `false`, telemetry events for crashes
would work very rarely. When set to `true`, this would become reliably
consistent.

The value for `wait_for_receiver` is [unset in
`libdatadog`](https://github.com/DataDog/libdatadog/blob/16528ffee456f7af5fe9ad80a6294fb5dcd38918/crashtracker/src/shared/configuration.rs#L28),
which means it is set to `false` by default. This default needs to be
updated on the `libdatadog` end. However, in order to unblock
crashtracking functionality for `dd-trace-py`, we are also enabling this
to be configured on the `dd-trace-py` side as well.

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)

(cherry picked from commit bb82a8e)
  • Loading branch information
erikayasuda authored and github-actions[bot] committed Aug 26, 2024
1 parent 03cfc4d commit 9d59731
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 1 deletion.
1 change: 1 addition & 0 deletions ddtrace/internal/core/crashtracking.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def start() -> bool:
crashtracker.set_runtime_id(get_runtime_id())
crashtracker.set_library_version(version.get_version())
crashtracker.set_alt_stack(bool(crashtracker_config.alt_stack))
crashtracker.set_wait_for_receiver(bool(crashtracker_config.wait_for_receiver))
if crashtracker_config.stacktrace_resolver == "fast":
crashtracker.set_resolve_frames_fast()
elif crashtracker_config.stacktrace_resolver == "full":
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ def set_library_version(profiler_version: StringType) -> None: ...
def set_stdout_filename(filename: StringType) -> None: ...
def set_stderr_filename(filename: StringType) -> None: ...
def set_alt_stack(alt_stack: bool) -> None: ...
def set_wait_for_receiver(wait: bool) -> None: ...
def set_resolve_frames_disable() -> None: ...
def set_resolve_frames_fast() -> None: ...
def set_resolve_frames_full() -> None: ...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ cdef extern from "crashtracker_interface.hpp":
void crashtracker_set_stdout_filename(string_view filename)
void crashtracker_set_stderr_filename(string_view filename)
void crashtracker_set_alt_stack(bint alt_stack)
void crashtracker_set_wait_for_receiver(bint wait)
void crashtracker_set_resolve_frames_disable()
void crashtracker_set_resolve_frames_fast()
void crashtracker_set_resolve_frames_full()
Expand Down Expand Up @@ -98,6 +99,10 @@ def set_alt_stack(alt_stack: bool) -> None:
crashtracker_set_alt_stack(alt_stack)


def set_wait_for_receiver(wait: bool) -> None:
crashtracker_set_wait_for_receiver(wait)


def set_resolve_frames_disable() -> None:
crashtracker_set_resolve_frames_disable()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class Crashtracker
{
private:
bool create_alt_stack = false;
bool wait_for_receiver = true;
std::optional<std::string> stderr_filename{ std::nullopt };
std::optional<std::string> stdout_filename{ std::nullopt };
std::string path_to_receiver_binary;
Expand Down Expand Up @@ -69,6 +70,7 @@ class Crashtracker
void set_library_version(std::string_view _library_version);
void set_url(std::string_view _url);
void set_tag(std::string_view _key, std::string_view _value);
void set_wait_for_receiver(bool _wait);

void set_create_alt_stack(bool _create_alt_stack);
void set_stderr_filename(std::string_view _stderr_filename);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ extern "C"
void crashtracker_set_stdout_filename(std::string_view filename);
void crashtracker_set_stderr_filename(std::string_view filename);
void crashtracker_set_alt_stack(bool alt_stack);
void crashtracker_set_wait_for_receiver(bool wait);
void crashtracker_set_resolve_frames_disable();
void crashtracker_set_resolve_frames_fast();
void crashtracker_set_resolve_frames_full();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ Datadog::Crashtracker::set_create_alt_stack(bool _create_alt_stack)
create_alt_stack = _create_alt_stack;
}

void
Datadog::Crashtracker::set_wait_for_receiver(bool _wait)
{
wait_for_receiver = _wait;
}

void
Datadog::Crashtracker::set_env(std::string_view _env)
{
Expand Down Expand Up @@ -122,7 +128,7 @@ Datadog::Crashtracker::get_config()
config.endpoint = ddog_prof_Endpoint_agent(to_slice(url));
config.resolve_frames = resolve_frames;
config.timeout_secs = timeout_secs;

config.wait_for_receiver = wait_for_receiver;
return config;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,12 @@ crashtracker_set_alt_stack(bool alt_stack) // cppcheck-suppress unusedFunction
crashtracker.set_create_alt_stack(alt_stack);
}

void
crashtracker_set_wait_for_receiver(bool wait) // cppcheck-suppress unusedFunction
{
crashtracker.set_wait_for_receiver(wait);
}

void
crashtracker_set_resolve_frames_disable() // cppcheck-suppress unusedFunction
{
Expand Down
8 changes: 8 additions & 0 deletions ddtrace/settings/crashtracker.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,5 +101,13 @@ class CrashtrackingConfig(En):
"This is generally useful only for dd-trace-py development.",
)

wait_for_receiver = En.v(
bool,
"wait_for_receiver",
default=True,
help_type="Boolean",
help="Whether to wait for the crashtracking receiver",
)


config = CrashtrackingConfig()

0 comments on commit 9d59731

Please sign in to comment.