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

[PROF-10275] Enable crashtracking telemetry by default when profiler is enabled #3816

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Jul 31, 2024

What does this PR do?

This PR enables the crashtracking telemetry feature by default when the profiler is enabled.

This feature can be disabled using the DD_PROFILING_EXPERIMENTAL_CRASH_TRACKING_ENABLED environment variable or the c.profiling.advanced.experimental_crash_tracking_enabled setting via code.

This setting is not very well-named, as the feature is no longer experimental. But as we're actually considering separating crashtracking from profiling, I decided to leave the current setting in place until we can decide where this is going to live, since that's going to influence where the new setting without "experimental" on its name should land.

Motivation:

Crashtracking allows us to identify issues that may otherwise go unnoticed. We've been deploying it on other Datadog libraries with great success so far.

Additional Notes:

We've had configurations running with crashtracking enabled on our usual validation environments + almost all (1 remaining!) internal Ruby apps also has been running with this feature.

We've not seen any noticeable impact for enabling this feature.

In fact, now that it's going to be the default, I'll go ahead and remove the separate configurations we had for testing it.

How to test the change?

The following snippet starts a Ruby instance with profiling, and crashes it immediately:

DD_PROFILING_ENABLED=true bundle exec ddprofrb exec ruby -e 'Process.kill("SEGV", Process.pid)'

You should see an error report submitted to the agent.

…is enabled

**What does this PR do?**

This PR enables the crashtracking telemetry feature by default when the
profiler is enabled.

This feature can be disabled using the
`DD_PROFILING_EXPERIMENTAL_CRASH_TRACKING_ENABLED` environment variable
or the `c.profiling.advanced.experimental_crash_tracking_enabled`
setting via code.

This setting is not very well-named, as the feature is no longer
experimental. But as we're actually considering separating crashtracking
from profiling, I decided to leave the current setting in place until we
can decide where this is going to live, since that's going to influence
where the new setting without "experimental" on its name should land.

**Motivation:**

Crashtracking allows us to identify issues that may otherwise go
unnoticed. We've been deploying it on other Datadog libraries with great
success so far.

**Additional Notes:**

We've had configurations running with crashtracking enabled on our usual
validation environments + almost all (1 remaining!) internal Ruby apps
also has been running with this feature.

We've not seen any noticeable impact for enabling this feature.

In fact, now that it's going to be the default, I'll go ahead and remove
the separate configurations we had for testing it.

**How to test the change?**

The following snippet starts a Ruby instance with profiling, and crashes
it immediately:

```bash
DD_PROFILING_ENABLED=true bundle exec ddprofrb exec ruby -e 'Process.kill("SEGV", Process.pid)'
```

You should see an error report submitted to the agent.
@ivoanjo ivoanjo requested review from a team as code owners July 31, 2024 09:54
@github-actions github-actions bot added core Involves Datadog core libraries profiling Involves Datadog profiling labels Jul 31, 2024
@pr-commenter
Copy link

pr-commenter bot commented Jul 31, 2024

Benchmarks

Benchmark execution time: 2024-07-31 10:22:06

Comparing candidate commit c686714 in PR branch ivoanjo/prof-10275-enable-crashtracker-profiler with baseline commit 47f12db in branch master.

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

scenario:Tracing.log_correlation

  • 🟥 throughput [-2837.581op/s; -2511.958op/s] or [-2.406%; -2.130%]

@ivoanjo
Copy link
Member Author

ivoanjo commented Aug 1, 2024

The one CI failure is being tracked separately and is unrelated to this PR (it's affecting all PRs for the past ~2 days).

I'm going to go ahead and merge this one in!

@ivoanjo ivoanjo merged commit 7059e5c into master Aug 1, 2024
180 of 181 checks passed
@ivoanjo ivoanjo deleted the ivoanjo/prof-10275-enable-crashtracker-profiler branch August 1, 2024 07:54
@github-actions github-actions bot added this to the 2.3.0 milestone Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Involves Datadog core libraries profiling Involves Datadog profiling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants