fix(profiling): fix type check for exception types using isinstance

[taegyunkim]

Previously, exc_type = <class Exception> would have failed the if check and now it passes as expected. This will result in showing exception type names in Timeline view in profiling, which is gated by a feature flag.

Tested using a sample application at https://github.com/DataDog/profiling-timeline-python-sample-app
before:

after:

Unfortunately, there's no easy way to write a Python unit test for this change. As the change touches a Cython file which should be a thin layer between a C++ extension and Python code. Also, we don't have any existing tests written in Cython that can be run using pytest, which could have been extended for this PR.

When DD_PROFILING_EXPORT_LIBDD_ENABLED is set and DD_PROFILING_STACK_V2_ENABLED is not set, the following chain of calls happens to collect exception information

• ddtrace.profiling.collector.stack.StackCollector.collect_stack link
• ddtrace.internal.datadog.profiling.ddup.SampleHandle.push_exceptioninfo link
• ddup_push_exceptioninfo (extern C function) link
• Datadog::Sample::push_exceptioninfo (C++ function) link

The only way to inspect the collected samples is using Datadog::Sample::profile_borrow() which isn't directly callable from a Python unit test.

Also, when STACK_V2 is enabled, this whole code path can be removed so I'd avoid spending too much time and effort in an interim solution, though I already did.

Checklist

• Change(s) are motivated and described in the PR description
• Testing strategy is described if automated tests are not included in the PR
• Risks are described (performance impact, potential for breakage, maintainability)
• Change is maintainable (easy to change, telemetry, documentation)
• Library release note guidelines are followed or label changelog/no-changelog is set
• Documentation is included (in-code, generated user docs, public corp docs)
• Backport labels are set (if applicable)
• If this PR changes the public interface, I've notified @DataDog/apm-tees.

Reviewer Checklist

• Title is accurate
• All changes are related to the pull request's stated goal
• Description motivates each change
• Avoids breaking API changes
• Testing strategy adequately addresses listed risks
• Change is maintainable (easy to change, telemetry, documentation)
• Release note makes sense to a user of the library
• 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

[datadog-dd-trace-py-rkomorn]

Datadog Report

Branch report: taegyunkim/prof-9923-python-labels
Commit report: 27a7be2
Test service: dd-trace-py

✅ 0 Failed, 207 Passed, 837 Skipped, 48m 13.79s Total duration (29m 26.38s time saved)

[pr-commenter]

Benchmarks

Benchmark execution time: 2024-06-18 22:39:46

Comparing candidate commit 8f511bbeaedf9923207c6721f453e401899d6e3a in PR branch taegyunkim/prof-9923-python-labels with baseline commit 16f3f95 in branch main.

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

[danielsn]

Do we have a regression test for this?

[taegyunkim]

I don't think we do. This is the commit/pr that introduced this if check.

interface.py looks like a test but I was not able to figure out how to invoke it.

[sanchda]

LGTM--thanks for fixing this

[taegyunkim]

/merge

[dd-devflow]

🚂 MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

Use /merge -c to cancel this operation!

[dd-devflow]

🚂 MergeQueue: This merge request was already merged

This pull request was merged directly.