Error logs remediation for Profiling native extension #5076

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

What does this PR do?

Notable caveats:

Errno didn't need to be changed: Errno errors encode their error information in the class name (which is already reported), thus there's no need to add new information for telemetry purposes.

Motivation:

Change log entry

Additional Notes:

How to test the change?

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 <[email protected]>

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

Benchmarks

Benchmark execution time: 2025-11-26 00:51:30

Comparing candidate commit 5432e65 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.

These files had extensive line wrapping/reformatting that has been reverted. Content changes (include reordering and commented rb_raise calls) have been preserved.

ivoanjo

Thanks for looking into this! Did a pass :)

ivoanjo · 2025-11-21T11:59:37Z

ext/datadog_profiling_native_extension/private_vm_api_access.c

+extern VALUE eNativeRuntimeError;
+extern VALUE eNativeArgumentError;
+extern VALUE eNativeTypeError;
+// Declare exception class globals from ruby_helpers.h
+// (We can't include ruby_helpers.h here as it pulls in public Ruby headers that conflict with private VM headers)
+#define raise_error(native_exception_class, fmt, ...) \
+  _raise_error(native_exception_class, "" fmt, ##__VA_ARGS__)
+NORETURN(
+  void _raise_error(VALUE native_exception_class, const char *fmt, ...)
+  __attribute__ ((format (printf, 2, 3)));
+);


Yeah this file is kinda... always a pain. TBH maybe it's easy to just not change anything here -- we'll miss exceptions from this file, but they're rare-ish so I'm not sure we care.

Either that or perhaps having a very simple extern raise_native_error(char *); that gets exported elsewhere and that this class uses directly (so this class doesn't get to pick the specific exception class or template stuff -- always get a native error + a static string)"?

ext/datadog_profiling_native_extension/profiling.c

ivoanjo · 2025-11-21T12:18:19Z

ext/datadog_profiling_native_extension/setup_signal_handler.c

  if (sigaction(SIGPROF, &signal_handler_config, &existing_signal_handler_config) != 0) {
-    rb_exc_raise(rb_syserr_new_str(errno, rb_sprintf("Could not install profiling signal handler (%s)", handler_pretty_name)));
+    raise_syserr(errno, true, "Could not install profiling signal handler", __FILE__, __LINE__, handler_pretty_name);
  }


Hmmm passing in handler_pretty_name as the function name argument to raise_syserr is... a bit weird. Is it a problem to template this string?

If needed, I'm thinking we can turn this into an enum -- we always know which handler_pretty_names are allowed, since it's all in the code anyway...

Yeah, this was like a the last call site I had to "fix" and I think I just tried to fit it in the existing methods.
I'll see I can try the enum so I don't bastardize it.

ivoanjo · 2025-11-21T12:19:02Z

ext/datadog_profiling_native_extension/setup_signal_handler.c

            handler_pretty_name
          )
        )
      );


⬆️ The rb_exc_raise on line 59 one possibly needs to change to match the others 👀

ivoanjo · 2025-11-21T12:20:55Z

ext/libdatadog_api/crashtracker.c

I'm guessing some of the shared error code may need to move to datadog_ruby_common.h/.c so it can be used here too...

ext/datadog_profiling_native_extension/ruby_helpers.c

ivoanjo · 2025-11-21T12:53:30Z

sig/datadog/profiling.rbs

I guess this is slightly outdated vs what's there in profiling.rb right now?

I didn't do all the janitorial work 😛. I'll keep this comment open as a reminder.

spec/datadog/profiling/collectors/code_provenance_spec.rb

…iler-code

github-actions · 2025-11-25T00:51:34Z

Typing analysis

Note: Ignored files are excluded from the next sections.

Untyped methods

This PR introduces 1 untyped method and 1 partially typed method, and clears 1 partially typed method. It decreases the percentage of typed methods from 54.48% to 54.47% (-0.01%).

Untyped methods (+1-0)

❌ Introduced:

sig/datadog/profiling.rbs:5
└── def initialize: (*untyped args, **untyped kwargs) -> void

Partially typed methods (+1-1)

❌ Introduced:

sig/datadog/profiling.rbs:20
└── def self.try_reading_skipped_reason_file: (?untyped file_api) -> ::String?

✅ Cleared:

sig/datadog/profiling.rbs:14
└── def self.try_reading_skipped_reason_file: (?untyped file_api) -> ::String?

If you believe a method or an attribute is rightfully untyped or partially typed, you can add # untyped:accept to the end of the line to remove it from the stats.

Signed-off-by: Marco Costa <[email protected]>

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 <[email protected]>

Second

d740503

wip

f14ee3a

wip

f4234c6

wip

b99a2cc

wip

c83b723

Signed-off-by: Marco Costa <[email protected]>

wip

90327d1

wip

d7f0dc3

Signed-off-by: Marco Costa <[email protected]>

wip

c191889

wip

639ee6e

wip

ef6b441

wip

5e6fc62

wip

19db45e

wip

962845d

Signed-off-by: Marco Costa <[email protected]>

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

Revert whitespace-only changes (blank lines)

6c77ebc

Revert line wrapping and whitespace changes in libdatadog_api files

b7edd96

These files had extensive line wrapping/reformatting that has been reverted. Content changes (include reordering and commented rb_raise calls) have been preserved.

marcotc force-pushed the marcotc/error-logs-remediation-custom-profiler-code branch from ef1db55 to b7edd96 Compare November 21, 2025 01:14

ivoanjo reviewed Nov 21, 2025

View reviewed changes

ivoanjo mentioned this pull request Nov 21, 2025

Add compile-time safe exception macros for profiler telemetry #4990

Open

marcotc added 4 commits November 24, 2025 14:59

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

1192a01

…iler-code

Rename prefix

c1cb858

Change arg type

2b260e6

Raise argument error exception

dd009d1

marcotc added 4 commits November 25, 2025 14:51

Move init

745a67f

Signed-off-by: Marco Costa <[email protected]>

Add fmt string

fe72ab0

Add telemetry str

4a2e3be

Add errno telemetry

5432e65

Error logs remediation for Profiling native extension #5076

Are you sure you want to change the base?

Error logs remediation for Profiling native extension #5076

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!

ivoanjo left a comment

Choose a reason for hiding this comment

Uh oh!

ivoanjo Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ivoanjo Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

marcotc Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

ivoanjo Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

ivoanjo Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ivoanjo Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

marcotc Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Nov 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Typing analysis

Untyped methods

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

github-actions bot commented Nov 25, 2025 •

edited

Loading