Telemetry-safe error reporting for native extensions #5076

marcotc · 2025-11-21T00:38:56Z

(I'll squash the commits on reviews finish)

What does this PR do?

This PR ensure errors raised from native ext/ code have enough context to be valuable in telemetry.

Motivation:

Because error information sent to telemetry cannot have arbitrary, dynamic data, we must ensure that we are only sending values that are known at gem build time.

Because our fallback is to only report the exception class, this PR adds a telemetry_message to native exceptions, so that their error information is not completely lost.

Change log entry

Yes. Telemetry: Added static error reporting for native extensions.

Additional Notes:

There's a follow up PR to reduce code duplication for the datadog_ruby_common.c/h files: #5088

How to test the change?
Easiest and fastest test it: bundle exec rake clean compile && bundle exec rake spec:profiling
There are more products supported by libdatadog, but at that point, just run CI :)

This gets us closer to allowing these errors to be sent to telemetry.

Add ruby_helpers.h include to 8 C files that use datadog_profiling_error_class and datadog_profiling_internal_error_class but were missing the header declaration. This fixes the compilation error: error: 'datadog_profiling_error_class' undeclared Files fixed: - clock_id_from_pthread.c - collectors_gc_profiling_helper.c - collectors_stack.c - collectors_thread_context.c - encoded_profile.c - libdatadog_helpers.c - private_vm_api_access.c - unsafe_api_calls_check.c

Move ruby_helpers.h include after private VM headers to avoid conflicts. This file requires private VM headers to be included first before any public Ruby headers, but ruby_helpers.h includes datadog_ruby_common.h which includes ruby.h, causing header ordering conflicts. Fixes compilation error: 'expected ')' before '==' token in RHASH_EMPTY_P'

Cannot include ruby_helpers.h in this file as it pulls in public Ruby headers (via datadog_ruby_common.h) that conflict with private VM headers. Instead, declare the exception class globals as extern, following the pattern already established in this file for other declarations. This fully resolves the header ordering compilation error.

Method was renamed from safe_exception_message to constant_exception_message but the RBS signature file was not updated, causing Steep type errors.

The error method must be public but was accidentally made private when constant_exception_message was added. Moving it before the private keyword restores its public visibility. Fixes test failure: NoMethodError: private method 'error' called

Serialization errors contain dynamic libdatadog content, so they should raise ProfilingInternalError (not ProfilingError or RuntimeError). Updated both the Ruby wrapper code and the test expectation to use ProfilingInternalError consistently. Fixes test failure expecting ProfilingError but getting RuntimeError.

…ler-code

Signed-off-by: Marco Costa <marco.costa@datadoghq.com>

pr-commenter · 2025-11-21T01:07:27Z

Benchmarks

Benchmark execution time: 2025-12-01 23:29:10

Comparing candidate commit f92ebb4 in PR branch marcotc/error-logs-remediation-custom-profiler-code with baseline commit ae94edc in branch master.

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

…remediation-custom-profiler-code' into marcotc/error-logs-remediation-custom-profiler-code

…iler-code

Signed-off-by: Marco Costa <marco.costa@datadoghq.com>

datadog-datadog-prod-us1 · 2025-12-08T19:55:07Z

✅ Tests

🎉 All green!

❄️ No new flaky tests detected
🧪 All tests passed

🎯 Code Coverage
• Patch Coverage: 88.20%
• Total Coverage: 95.23% (-0.00%)

View detailed report

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 976a297 | Docs | Datadog PR Page | Was this helpful? Give us feedback!}

…iler-code

ivoanjo · 2025-12-11T10:48:52Z

The failure in the benchmarks...

+ bundle exec ruby benchmarks/profiling_sample_gvl.rb
Current pid is 3296
ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [x86_64-linux]
Warming up --------------------------------------
profiling - gvl benchmark samples
benchmarks/profiling_sample_gvl.rb:64:in `_native_sample_after_gvl_running': wrong number of arguments (given 3, expected 2) (ArgumentError)
        Datadog::Profiling::Collectors::ThreadContext::Testing._native_sample_after_gvl_running(@collector, @target_thread, false)
                                                                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	from benchmarks/profiling_sample_gvl.rb:64:in `block (2 levels) in run_benchmark'
	from (eval at /usr/local/bundle/gems/benchmark-ips-2.14.0/lib/benchmark/ips/job/entry.rb:63):6:in `call_times'
	from /usr/local/bundle/gems/benchmark-ips-2.14.0/lib/benchmark/ips/job.rb:285:in `block in run_warmup'
	from /usr/local/bundle/gems/benchmark-ips-2.14.0/lib/benchmark/ips/job.rb:268:in `each'
	from /usr/local/bundle/gems/benchmark-ips-2.14.0/lib/benchmark/ips/job.rb:268:in `run_warmup'
	from /usr/local/bundle/gems/benchmark-ips-2.14.0/lib/benchmark/ips/job.rb:253:in `block in run'

is because the "use the test code from this branch with master" doesn't work if master isn't API-compatible with this branch.

TBH what I've done in the past is just disable the benchmark and re-enable it later, but it's kinda meh workaround. Suggestions welcome 😅

ivoanjo

This is looking great! The one reason I didn't press the approve button is due to the raise_error in datadog_ruby_common.c -- that one doesn't quite look correct.

ivoanjo · 2025-12-11T10:57:36Z

ext/datadog_profiling_native_extension/datadog_ruby_common.c

+void datadog_ruby_common_init(VALUE datadog_module) {
+  // No longer needed - using Ruby's built-in exception classes
+  (void)datadog_module;
+}


I'd suggest removing this + its callsites; it's a bit of complexity (including comments in "profiling.c" saying this needs to be called first) and I'm not sure it's worth leaving around.

ivoanjo · 2025-12-11T11:04:01Z

ext/datadog_profiling_native_extension/datadog_ruby_common.c

+void raise_error(VALUE error_class, const char *fmt, ...) {
+  va_list args;
+  va_start(args, fmt);
+  VALUE message = rb_vsprintf(fmt, args);
+  va_end(args);
+  rb_raise(error_class, "%"PRIsVALUE, message);
+}


Wait, I'm confused by this... Is this a leftover from an earlier version?

That is, I thought the correct version of raise_error was the #define raise_error we have in ruby_helpers.c (the one that grabs the format string separately, etc); this one seems to just be a weird direct passthrough to rb_raise.

Am I missing something? 🤔 👀

ivoanjo · 2025-12-11T11:11:37Z

spec/datadog/profiling/native_extension_spec.rb

+      it "is an instance of RuntimeError" do
+        expect { raise_native_runtime_error }.to raise_exception(RuntimeError)
+      end
+    end
+
+    context "when raising ArgumentError" do
+      subject(:raise_native_argument_error) do
+        described_class::Testing._native_grab_gvl_and_raise(::ArgumentError, "argument error test", nil, true)
+      end
+
+      it "raises an ArgumentError" do
+        expect { raise_native_argument_error }.to raise_error(::ArgumentError) do |error|
+          expect(error.message).to eq("argument error test")
+          expect(error.instance_variable_get(:@telemetry_message)).to eq("argument error test")
+        end
+      end
+
+      it "is an instance of ArgumentError" do
+        expect { raise_native_argument_error }.to raise_exception(ArgumentError)
+      end
+    end
+
+    context "when raising TypeError" do
+      subject(:raise_native_type_error) do
+        described_class::Testing._native_grab_gvl_and_raise(::TypeError, "type error test", nil, true)
+      end
+
+      it "raises a TypeError" do
+        expect { raise_native_type_error }.to raise_error(::TypeError) do |error|
+          expect(error.message).to eq("type error test")
+          expect(error.instance_variable_get(:@telemetry_message)).to eq("type error test")
+        end
+      end
+
+      it "is an instance of TypeError" do
+        expect { raise_native_type_error }.to raise_exception(TypeError)
      end


Minor: The "is an instance of" tests can be removed, since the raise_error(...) validations in the "it raises ..." testcases already include that.

ivoanjo · 2025-12-11T11:13:01Z

ext/datadog_profiling_native_extension/ruby_helpers.h

 #define ENFORCE_SUCCESS_HELPER(expression, have_gvl) \
-  { int result_syserr_errno = expression; if (RB_UNLIKELY(result_syserr_errno)) raise_syserr(result_syserr_errno, have_gvl, ADD_QUOTES(expression), __FILE__, __LINE__, __func__); }
+  { int result_syserr_errno = expression; if (RB_UNLIKELY(result_syserr_errno)) raise_enforce_syserr(result_syserr_errno, have_gvl, ADD_QUOTES(expression), __FILE__, __LINE__, __func__); }

 #define RUBY_NUM_OR_NIL(val, condition, conv) ((val condition) ? conv(val) : Qnil)
 #define RUBY_AVG_OR_NIL(total, count) ((count == 0) ? Qnil : DBL2NUM(((double) total) / count))

 // Called by ENFORCE_SUCCESS_HELPER; should not be used directly
-NORETURN(void raise_syserr(
+NORETURN(void raise_enforce_syserr(


Minor: To match the private naming we used above, maybe these should become private_raise_syserr?

ivoanjo · 2025-12-11T11:14:55Z

ext/datadog_profiling_native_extension/ruby_helpers.c

+// Make sure to *not* invoke Ruby code as this function can run in unsafe contexts.
+// NOTE: Raising the exception acquires the GVL (unsafe), but it also aborts the current execution flow.
+// @see debug_enter_unsafe_context


Suggested change

// Make sure to *not* invoke Ruby code as this function can run in unsafe contexts.

// NOTE: Raising the exception acquires the GVL (unsafe), but it also aborts the current execution flow.

// @see debug_enter_unsafe_context

// NOTE: Raising an exception always invokes Ruby code so it requires the GVL and is not compatible with "debug_enter_unsafe_context".

// @see debug_enter_unsafe_context

bouwkast and others added 26 commits November 7, 2025 11:40

Implement custom exception approach for errors from C code

8e2bf94

Update to support constant_exception_message and ProfilingError

27067c0

This gets us closer to allowing these errors to be sent to telemetry.

Implement two-tier exception strategy for profiler errors

3d908d7

Update RBS signature for renamed method constant_exception_message

abe3f0d

Method was renamed from safe_exception_message to constant_exception_message but the RBS signature file was not updated, causing Steep type errors.

Add compile-time safe macros for profiler exception raising

c31665f

Merge branch 'master' into steven/error-logs-remediation-custom-profi…

6a292a4

…ler-code

Working

b5480a1

First try

807af7b

Signed-off-by: Marco Costa <marco.costa@datadoghq.com>

Second

d740503

wip

f14ee3a

wip

f4234c6

wip

b99a2cc

wip

c83b723

Signed-off-by: Marco Costa <marco.costa@datadoghq.com>

wip

90327d1

wip

d7f0dc3

Signed-off-by: Marco Costa <marco.costa@datadoghq.com>

wip

c191889

wip

639ee6e

wip

ef6b441

wip

5e6fc62

wip

19db45e

wip

962845d

Signed-off-by: Marco Costa <marco.costa@datadoghq.com>

github-actions bot added core Involves Datadog core libraries profiling Involves Datadog profiling labels Nov 21, 2025

Revert whitespace-only changes (blank lines)

6c77ebc

marcotc added 17 commits December 3, 2025 13:42

Remove unused syserr_raise_args struct

4cb395a

Remove rb_class2name

d2ac473

Add better init comments

5981b7c

Remove inline

b210b40

Sync files

a041560

Add test

e0064a3

Merge remote-tracking branch 'refs/remotes/origin/marcotc/error-logs-…

6183bb7

…remediation-custom-profiler-code' into marcotc/error-logs-remediation-custom-profiler-code

Merge branch 'master' into marcotc/error-logs-remediation-custom-prof…

7a99d1e

…iler-code

Merge branch 'master' into marcotc/error-logs-remediation-custom-prof…

115adc1

…iler-code

Remove custom classes

9afc7c4

Modify FeatureFlags

9f7516d

wip

3b36017

wip

065f694

wip

1db9222

wip

883eff3

wip

7c42a29

wip

6a6bdfb

marcotc force-pushed the marcotc/error-logs-remediation-custom-profiler-code branch from b2d87f4 to 6a6bdfb Compare December 5, 2025 23:43

marcotc added 3 commits December 5, 2025 15:53

wip

d19db29

Fix compiliation warning

092d8cd

Fix rbs

f99ff3d

marcotc marked this pull request as ready for review December 6, 2025 00:03

marcotc requested review from a team as code owners December 6, 2025 00:03

Remove unsued

824a340

Signed-off-by: Marco Costa <marco.costa@datadoghq.com>

Merge branch 'master' into marcotc/error-logs-remediation-custom-prof…

976a297

…iler-code

marcotc self-assigned this Dec 8, 2025

ivoanjo reviewed Dec 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Telemetry-safe error reporting for native extensions #5076

Telemetry-safe error reporting for native extensions #5076

Uh oh!

marcotc commented Nov 21, 2025 •

edited

Loading

Uh oh!

pr-commenter bot commented Nov 21, 2025 •

edited

Loading

Uh oh!

datadog-datadog-prod-us1 bot commented Dec 8, 2025 •

edited

Loading

Uh oh!

ivoanjo commented Dec 11, 2025

Uh oh!

ivoanjo left a comment

Uh oh!

ivoanjo Dec 11, 2025

Uh oh!

ivoanjo Dec 11, 2025

Uh oh!

ivoanjo Dec 11, 2025

Uh oh!

ivoanjo Dec 11, 2025

Uh oh!

ivoanjo Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Telemetry-safe error reporting for native extensions #5076

Are you sure you want to change the base?

Telemetry-safe error reporting for native extensions #5076

Uh oh!

Conversation

marcotc commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pr-commenter bot commented Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks

Uh oh!

datadog-datadog-prod-us1 bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ivoanjo commented Dec 11, 2025

Uh oh!

ivoanjo left a comment

Choose a reason for hiding this comment

Uh oh!

ivoanjo Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

ivoanjo Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

ivoanjo Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

ivoanjo Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

ivoanjo Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

marcotc commented Nov 21, 2025 •

edited

Loading

pr-commenter bot commented Nov 21, 2025 •

edited

Loading

datadog-datadog-prod-us1 bot commented Dec 8, 2025 •

edited

Loading