Add fallback name for main thread in profiling data #2939
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
**What does this PR do?**: This PR adds a fallback name of "main" to the main Ruby thread. The main Ruby thread is first thread created in the VM, and the one that is returned by `Thread.main` (when in the main Ractor). **Motivation**: As more profiling features start to show the name in the UX, it's annoying to often have the main thread not have a name. **Additional Notes**: This PR sits atop #2937 to avoid any merge conflicts (since both touch labels and the `ThreadContext` collector state), but is otherwise completely independent of that one. **How to test the change?**: This change includes test coverage. You can see it in action by profiling a Ruby app that never names its main thread, and then confirming that profiling data for that thread is shown under the name `main` in the Datadog UX.
gcc version 10.2.1 20210110 (Debian 10.2.1-6), used on our Ruby 2.7
images at time of writing, was incorrectly triggering a warning on this
code, and since in CI we compile with `-Werror` (we turn warnings into
errors), this was breaking.
This is not the first time we've seen gcc tripping around structure
initializers, so I'm not entirely surprised.
Shuffling the code around a bit made the warning go away.
Full details:
```
compiling ../../../../ext/ddtrace_profiling_native_extension/collectors_thread_context.c
../../../../ext/ddtrace_profiling_native_extension/collectors_thread_context.c: In function 'trigger_sample_for_thread':
../../../../ext/ddtrace_profiling_native_extension/collectors_thread_context.c:661:5: error: missing initializer for field 'num' of 'ddog_prof_Label' [-Werror=missing-field-initializers]
661 | };
| ^
In file included from /usr/local/bundle/gems/libdatadog-2.0.0.1.0-x86_64-linux/vendor/libdatadog-2.0.0/x86_64-linux/libdatadog-x86_64-unknown-linux-gnu/lib/pkgconfig/../../include/datadog/profiling.h:12,
from ../../../../ext/ddtrace_profiling_native_extension/collectors_stack.h:3,
from ../../../../ext/ddtrace_profiling_native_extension/collectors_thread_context.c:5:
/usr/local/bundle/gems/libdatadog-2.0.0.1.0-x86_64-linux/vendor/libdatadog-2.0.0/x86_64-linux/libdatadog-x86_64-unknown-linux-gnu/lib/pkgconfig/../../include/datadog/common.h:389:11: note: 'num' declared here
389 | int64_t num;
| ^~~
cc1: all warnings being treated as errors
```
Codecov Report
@@ Coverage Diff @@
## master #2939 +/- ##
==========================================
- Coverage 98.00% 98.00% -0.01%
==========================================
Files 1282 1282
Lines 71105 71111 +6
Branches 3277 3277
==========================================
+ Hits 69686 69691 +5
- Misses 1419 1420 +1
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?:
This PR adds a fallback name of "main" to the main Ruby thread.
The main Ruby thread is first thread created in the VM, and the one that is returned by
Thread.main(when in the main Ractor).Motivation:
As more profiling features start to show the name in the UX, it's annoying to often have the main thread not have a name.
Additional Notes:
This PR sits atop #2937 to avoid any merge conflicts (since both touch labels and the
ThreadContextcollector state), but is otherwise completely independent of that one.How to test the change?:
This change includes test coverage.
You can see it in action by profiling a Ruby app that never names its main thread, and then confirming that profiling data for that thread is shown under the name
mainin the Datadog UX.