[crashtracker] Gate compilation using cargo features #541
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
danielsn
force-pushed
the
dsn/crashtracker-cfg-features
branch
from
74f236f
to
08aa381
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #541 +/- ##
==========================================
- Coverage 70.45% 70.45% -0.01%
==========================================
Files 208 213 +5
Lines 28414 28408 -6
==========================================
- Hits 20020 20014 -6
Misses 8394 8394
|
BenchmarksThis comment was omitted because it was over 65536 characters.Please check the Gitlab Job logs to see its output. |
taegyunkim
previously approved these changes
Jul 19, 2024
taegyunkim
dismissed
their stale review
Looks like this also needs changes to ffi
sanchda
approved these changes
Jul 22, 2024
| @@ -1,12 +1,13 @@ | |||
| // Copyright 2021-Present Datadog, Inc. https://www.datadoghq.com/ | |||
There was a problem hiding this comment.
Silly question: do these copyright notices indicate the year of first existence for the noted file, or the last update?
There was a problem hiding this comment.
I assume first year of existence. At Amazon we stopped using years, and went to a SPDX header which was much cleaner
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?
Adds two new cargo features:
collectorandreceiver. Refactors the code to make it easy to use thosecfgto define what gets built at compile time.Motivation
The current implementation of the library always builds in all code. This is not great, because it bloats binary size etc. We have users who
CrashInfoobjects, which they upload. They have no need or use for thereceiverorcollectorportions of this codeCrashInfoobject and send it on. They need all parts of the code.CrashInfoobject and send it on. The first binary needs thecollectorbut not thereceiver; the sidecar needs the opposite.Additional Notes
Currently, we don't have support when building the ffi library for feature selection. This is a first step towards making that worthwhile.
I did some refactoring as I moved code around, but there is still more refactoring that could be done.
How to test the change?
This is a refactor, existing tests cover the functionality
PROF-10245