Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: enable data-pipeline feature in shipped artifacts #751

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

ganeshnj
Copy link
Contributor

@ganeshnj ganeshnj commented Nov 21, 2024

What does this PR do?

With this change, we will start shipping the data-pipeline code in the shipped artifacts.

I also made some refactor to make it easier to make such changes in future so we don't make mistake like cbindgen,datadog-profiling-ffi/ddtelemetry-ffi was included multiple times in past at

FEATURES="--features crashtracker-collector,crashtracker-receiver,cbindgen,datadog-profiling-ffi/ddtelemetry-ffi,cbindgen,datadog-profiling-ffi/ddtelemetry-ffi,datadog-profiling-ffi/demangler,symbolizer"

Motivation

data-pipeline integration in .NET SDK requires data-pipeline stuff as per DataDog/dd-trace-dotnet#6314, hence we need to start shipping the code so integrations can use it

How to test the change?

on mac

❯ ./build-profiling-ffi.sh bin
<truncated>
Building for features: cbindgen,crashtracker-collector,crashtracker-receiver,data-pipeline-ffi,datadog-profiling-ffi/ddtelemetry-ffi,datadog-profiling-ffi/demangler
<truncated>
[ 50%] Building C object CMakeFiles/libdatadog-crashtracking-receiver.dir/libdatadog-crashtracking-receiver.c.o
[100%] Linking C executable libdatadog-crashtracking-receiver
ld: warning: ignoring duplicate libraries: '-lSystem', '-liconv'
[100%] Built target libdatadog-crashtracking-receiver
Done.

on windows

❯ .\windows\build-artifacts.ps1 bin
<truncated>
Building for features: data-pipeline-ffi,datadog-profiling-ffi/crashtracker-collector,datadog-profiling-ffi/crashtracker-receiver,datadog-profiling-ffi/ddtelemetry-ffi,datadog-profiling-ffi/demangler
<truncated>
WARN: Missing `[defines]` entry for `feature = "receiver"` in cbindgen config.
WARN: Missing `[defines]` entry for `unix` in cbindgen config.
WARN: Missing `[defines]` entry for `feature = "receiver"` in cbindgen config.
Build finished

@ganeshnj ganeshnj marked this pull request as ready for review November 21, 2024 17:04
@ganeshnj ganeshnj requested review from a team as code owners November 21, 2024 17:04
Copy link
Contributor

@gleocadie gleocadie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.55%. Comparing base (c6ad4ff) to head (1200d9c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #751      +/-   ##
==========================================
- Coverage   70.59%   70.55%   -0.04%     
==========================================
  Files         296      296              
  Lines       43246    43246              
==========================================
- Hits        30528    30512      -16     
- Misses      12718    12734      +16     
Components Coverage Δ
crashtracker 37.14% <ø> (ø)
crashtracker-ffi 8.67% <ø> (ø)
datadog-alloc 98.73% <ø> (ø)
data-pipeline 91.98% <ø> (ø)
data-pipeline-ffi 0.00% <ø> (ø)
ddcommon 83.46% <ø> (ø)
ddcommon-ffi 69.12% <ø> (ø)
ddtelemetry 59.05% <ø> (ø)
ddtelemetry-ffi 22.13% <ø> (ø)
dogstatsd 89.45% <ø> (ø)
dogstatsd-client 79.77% <ø> (ø)
ipc 82.86% <ø> (+0.10%) ⬆️
profiling 84.30% <ø> (ø)
profiling-ffi 77.46% <ø> (ø)
serverless 0.00% <ø> (ø)
sidecar 38.01% <ø> (ø)
sidecar-ffi 0.00% <ø> (ø)
spawn-worker 50.36% <ø> (ø)
tinybytes 94.77% <ø> (ø)
trace-mini-agent 72.36% <ø> (ø)
trace-normalization 98.23% <ø> (ø)
trace-obfuscation 95.77% <ø> (ø)
trace-protobuf 77.67% <ø> (ø)
trace-utils 93.54% <ø> (ø)
---- 🚨 Try these New Features:

Copy link
Contributor

@ekump ekump left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.NET build artifacts are not my area of expertise, but LGTM

@gleocadie
Copy link
Contributor

.NET build artifacts are not my area of expertise, but LGTM

I'm here for that ;)

@ganeshnj ganeshnj merged commit 580c2f4 into main Nov 22, 2024
32 checks passed
@ganeshnj ganeshnj deleted the ganeshnj/chore/enable-data-pipeline-artifacts branch November 22, 2024 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants