From f55b72072fbaadec7692d7b85ec8a26acbc0e841 Mon Sep 17 00:00:00 2001 From: paullegranddc <82819397+paullegranddc@users.noreply.github.com> Date: Tue, 26 Sep 2023 15:28:20 +0200 Subject: [PATCH 1/6] Add a metric inteval and set it to 10s (#263) --- ddtelemetry/src/data/metrics.rs | 1 + ddtelemetry/src/metrics.rs | 2 ++ ddtelemetry/src/worker/mod.rs | 3 ++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ddtelemetry/src/data/metrics.rs b/ddtelemetry/src/data/metrics.rs index eb756d5db..726989e61 100644 --- a/ddtelemetry/src/data/metrics.rs +++ b/ddtelemetry/src/data/metrics.rs @@ -13,6 +13,7 @@ pub struct Serie { pub common: bool, #[serde(rename = "type")] pub _type: MetricType, + pub interval: u64, } #[derive(Serialize, Debug, Clone, Copy)] diff --git a/ddtelemetry/src/metrics.rs b/ddtelemetry/src/metrics.rs index 4d3c08b46..b487bfd2b 100644 --- a/ddtelemetry/src/metrics.rs +++ b/ddtelemetry/src/metrics.rs @@ -71,6 +71,8 @@ pub struct MetricBuckets { } impl MetricBuckets { + pub const METRICS_FLUSH_INTERVAL: time::Duration = time::Duration::from_secs(10); + pub fn flush_agregates(&mut self) { let timestamp = unix_timestamp_now(); for (key, bucket) in self.buckets.drain() { diff --git a/ddtelemetry/src/worker/mod.rs b/ddtelemetry/src/worker/mod.rs index 563391c50..f32e1ff55 100644 --- a/ddtelemetry/src/worker/mod.rs +++ b/ddtelemetry/src/worker/mod.rs @@ -401,6 +401,7 @@ impl TelemetryWorker { points, common: context.common, _type: context.metric_type, + interval: MetricBuckets::METRICS_FLUSH_INTERVAL.as_secs(), }); } @@ -790,7 +791,7 @@ impl TelemetryWorkerBuilder { client, deadlines: scheduler::Scheduler::new(vec![ ( - time::Duration::from_secs(10), + MetricBuckets::METRICS_FLUSH_INTERVAL, LifecycleAction::FlushMetricAggr, ), (telemetry_hearbeat_interval, LifecycleAction::FlushData), From 8712cad31ce4d3913de9df65ab0d9eeef4c22450 Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Tue, 26 Sep 2023 11:22:52 -0400 Subject: [PATCH 2/6] refactor(profiling): simplify directory structure (#258) --- profiling-ffi/src/exporter.rs | 4 +- profiling-ffi/src/profiles.rs | 31 +-- profiling-replayer/src/main.rs | 13 +- profiling-replayer/src/profile_index.rs | 2 +- profiling-replayer/src/replayer.rs | 4 +- profiling/examples/profiles.rs | 11 +- profiling/src/{profile => }/api.rs | 2 +- profiling/src/exporter/mod.rs | 2 +- .../endpoint_stats.rs} | 0 .../src/{profile => }/internal/endpoints.rs | 1 - .../src/{profile => }/internal/function.rs | 0 profiling/src/{profile => }/internal/label.rs | 0 .../src/{profile => }/internal/location.rs | 0 .../src/{profile => }/internal/mapping.rs | 0 profiling/src/{profile => }/internal/mod.rs | 6 + .../{profile => }/internal/observation/mod.rs | 0 .../internal/observation/observations.rs | 4 +- .../observation/trimmed_observation.rs | 0 .../{profile/mod.rs => internal/profile.rs} | 180 +++++++----------- .../src/{profile => }/internal/sample.rs | 0 .../src/{profile => }/internal/stack_trace.rs | 0 profiling/src/internal/timestamp.rs | 4 + .../src/{profile => }/internal/upscaling.rs | 6 +- .../src/{profile => }/internal/value_type.rs | 0 profiling/src/lib.rs | 4 +- profiling/src/{profile => }/pprof/mod.rs | 0 .../src/{profile => }/pprof/profile.proto | 0 profiling/src/{profile => }/pprof/proto.rs | 2 +- .../src/{profile => }/pprof/sliced_proto.rs | 0 .../src/{profile => }/pprof/test_utils.rs | 2 +- profiling/tests/wordpress.rs | 3 +- 31 files changed, 121 insertions(+), 160 deletions(-) rename profiling/src/{profile => }/api.rs (99%) rename profiling/src/{profile/profiled_endpoints.rs => internal/endpoint_stats.rs} (100%) rename profiling/src/{profile => }/internal/endpoints.rs (92%) rename profiling/src/{profile => }/internal/function.rs (100%) rename profiling/src/{profile => }/internal/label.rs (100%) rename profiling/src/{profile => }/internal/location.rs (100%) rename profiling/src/{profile => }/internal/mapping.rs (100%) rename profiling/src/{profile => }/internal/mod.rs (85%) rename profiling/src/{profile => }/internal/observation/mod.rs (100%) rename profiling/src/{profile => }/internal/observation/observations.rs (99%) rename profiling/src/{profile => }/internal/observation/trimmed_observation.rs (100%) rename profiling/src/{profile/mod.rs => internal/profile.rs} (93%) rename profiling/src/{profile => }/internal/sample.rs (100%) rename profiling/src/{profile => }/internal/stack_trace.rs (100%) create mode 100644 profiling/src/internal/timestamp.rs rename profiling/src/{profile => }/internal/upscaling.rs (98%) rename profiling/src/{profile => }/internal/value_type.rs (100%) rename profiling/src/{profile => }/pprof/mod.rs (100%) rename profiling/src/{profile => }/pprof/profile.proto (100%) rename profiling/src/{profile => }/pprof/proto.rs (99%) rename profiling/src/{profile => }/pprof/sliced_proto.rs (100%) rename profiling/src/{profile => }/pprof/test_utils.rs (88%) diff --git a/profiling-ffi/src/exporter.rs b/profiling-ffi/src/exporter.rs index 96725069a..1f46de8b4 100644 --- a/profiling-ffi/src/exporter.rs +++ b/profiling-ffi/src/exporter.rs @@ -7,7 +7,7 @@ use crate::Timespec; use datadog_profiling::exporter; use datadog_profiling::exporter::{ProfileExporter, Request}; -use datadog_profiling::profile::profiled_endpoints; +use datadog_profiling::internal::ProfiledEndpointsStats; use ddcommon::tag::Tag; use ddcommon_ffi::slice::{AsBytes, ByteSlice, CharSlice, Slice}; use ddcommon_ffi::Error; @@ -227,7 +227,7 @@ pub unsafe extern "C" fn ddog_prof_Exporter_Request_build( files_to_compress_and_export: Slice, files_to_export_unmodified: Slice, optional_additional_tags: Option<&ddcommon_ffi::Vec>, - optional_endpoints_stats: Option<&profiled_endpoints::ProfiledEndpointsStats>, + optional_endpoints_stats: Option<&ProfiledEndpointsStats>, optional_internal_metadata_json: Option<&CharSlice>, timeout_ms: u64, ) -> RequestBuildResult { diff --git a/profiling-ffi/src/profiles.rs b/profiling-ffi/src/profiles.rs index c7dbf85c1..9994bf6ea 100644 --- a/profiling-ffi/src/profiles.rs +++ b/profiling-ffi/src/profiles.rs @@ -2,7 +2,9 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc. use crate::Timespec; -use datadog_profiling::profile::{self, api, profiled_endpoints}; +use datadog_profiling::api; +use datadog_profiling::internal; +use datadog_profiling::internal::ProfiledEndpointsStats; use ddcommon_ffi::slice::{AsBytes, CharSlice, Slice}; use ddcommon_ffi::Error; use std::convert::{TryFrom, TryInto}; @@ -16,17 +18,17 @@ use std::time::{Duration, SystemTime}; pub struct Profile { // This may possibly be null, but will be a valid pointer to an owned // Profile otherwise. - inner: *mut profile::Profile, + inner: *mut internal::Profile, } impl Profile { - fn new(profile: profile::Profile) -> Self { + fn new(profile: internal::Profile) -> Self { Profile { inner: Box::into_raw(Box::new(profile)), } } - fn take(&mut self) -> Option> { + fn take(&mut self) -> Option> { // Leaving a null will help with double-free issues that can // arise in C. Of course, it's best to never get there in the // first place! @@ -349,13 +351,12 @@ pub unsafe extern "C" fn ddog_prof_Profile_new( start_time: Option<&Timespec>, ) -> ProfileNewResult { let types: Vec = sample_types.into_slice().iter().map(Into::into).collect(); + let start_time = start_time.map_or_else(SystemTime::now, SystemTime::from); + let period = period.map(Into::into); - let builder = profile::Profile::builder() - .period(period.map(Into::into)) - .sample_types(types) - .start_time(start_time.map(SystemTime::from)); - - ProfileNewResult::Ok(Profile::new(builder.build())) + let internal_profile = internal::Profile::new(start_time, &types, period); + let ffi_profile = Profile::new(internal_profile); + ProfileNewResult::Ok(ffi_profile) } /// # Safety @@ -433,7 +434,7 @@ unsafe fn ddog_prof_profile_add_impl( unsafe fn profile_ptr_to_inner<'a>( profile_ptr: *mut Profile, -) -> anyhow::Result<&'a mut profile::Profile> { +) -> anyhow::Result<&'a mut internal::Profile> { match profile_ptr.as_mut() { None => anyhow::bail!("profile pointer was null"), Some(inner_ptr) => match inner_ptr.inner.as_mut() { @@ -618,7 +619,7 @@ pub unsafe extern "C" fn ddog_prof_Profile_add_upscaling_rule_proportional( } unsafe fn add_upscaling_rule( - profile: &mut profile::Profile, + profile: &mut internal::Profile, offset_values: Slice, label_name: CharSlice, label_value: CharSlice, @@ -642,7 +643,7 @@ pub struct EncodedProfile { start: Timespec, end: Timespec, buffer: ddcommon_ffi::Vec, - endpoints_stats: Box, + endpoints_stats: Box, } /// # Safety @@ -659,8 +660,8 @@ pub unsafe extern "C" fn ddog_prof_EncodedProfile_drop(profile: Option<&mut Enco } } -impl From for EncodedProfile { - fn from(value: profile::EncodedProfile) -> Self { +impl From for EncodedProfile { + fn from(value: internal::EncodedProfile) -> Self { let start = value.start.into(); let end = value.end.into(); let buffer = value.buffer.into(); diff --git a/profiling-replayer/src/main.rs b/profiling-replayer/src/main.rs index 17ae98411..7eae2ded3 100644 --- a/profiling-replayer/src/main.rs +++ b/profiling-replayer/src/main.rs @@ -5,7 +5,6 @@ mod profile_index; mod replayer; use clap::{command, Arg, ArgAction}; -use datadog_profiling::profile; use prost::Message; use std::borrow::Cow; use std::io::Cursor; @@ -144,15 +143,15 @@ fn main() -> anyhow::Result<()> { std::fs::read(input)? }; - let pprof = profile::pprof::Profile::decode(&mut Cursor::new(source))?; + let pprof = datadog_profiling::pprof::Profile::decode(&mut Cursor::new(source))?; let mut replayer = Replayer::try_from(&pprof)?; - let mut outprof = profile::Profile::builder() - .start_time(Some(replayer.start_time)) - .sample_types(replayer.sample_types.clone()) - .period(replayer.period) - .build(); + let mut outprof = datadog_profiling::internal::Profile::new( + replayer.start_time, + &replayer.sample_types, + replayer.period, + ); // Before benchmarking, let's calculate some statistics. // No point doing that if there aren't at least 4 samples though. diff --git a/profiling-replayer/src/profile_index.rs b/profiling-replayer/src/profile_index.rs index b72bc9d24..6961f1c95 100644 --- a/profiling-replayer/src/profile_index.rs +++ b/profiling-replayer/src/profile_index.rs @@ -2,7 +2,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2023-Present Datadog, Inc. use anyhow::Result; -use datadog_profiling::profile::pprof::{Function, Location, Mapping, Profile}; +use datadog_profiling::pprof::{Function, Location, Mapping, Profile}; use std::collections::HashMap; pub struct ProfileIndex<'pprof> { diff --git a/profiling-replayer/src/replayer.rs b/profiling-replayer/src/replayer.rs index 7779737be..f9eae5821 100644 --- a/profiling-replayer/src/replayer.rs +++ b/profiling-replayer/src/replayer.rs @@ -2,7 +2,9 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2023-Present Datadog, Inc. use crate::profile_index::ProfileIndex; -use datadog_profiling::profile::{api, pprof, Timestamp}; +use datadog_profiling::api; +use datadog_profiling::internal::Timestamp; +use datadog_profiling::pprof; use std::ops::{Add, Sub}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; diff --git a/profiling/examples/profiles.rs b/profiling/examples/profiles.rs index ceae17a5b..2b9e9c04c 100644 --- a/profiling/examples/profiles.rs +++ b/profiling/examples/profiles.rs @@ -1,9 +1,11 @@ // Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc. -use datadog_profiling::profile::{api, Profile}; +use datadog_profiling::api; +use datadog_profiling::internal::Profile; use std::io::Write; use std::process::exit; +use std::time::SystemTime; // Keep this in-sync with profiles.c fn main() { @@ -54,11 +56,8 @@ fn main() { labels: vec![], }; - // Not setting .start_time intentionally to use the current time. - let mut profile: Profile = Profile::builder() - .sample_types(sample_types) - .period(Some(period)) - .build(); + // Intentionally use the current time. + let mut profile = Profile::new(SystemTime::now(), &sample_types, Some(period)); match profile.add(sample, None) { Ok(_) => {} diff --git a/profiling/src/profile/api.rs b/profiling/src/api.rs similarity index 99% rename from profiling/src/profile/api.rs rename to profiling/src/api.rs index 21d1e39a8..6d56dfc72 100644 --- a/profiling/src/profile/api.rs +++ b/profiling/src/api.rs @@ -1,7 +1,7 @@ // Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc. -use crate::profile::pprof; +use crate::pprof; use std::ops::{Add, Sub}; use std::time::{Duration, SystemTime, UNIX_EPOCH}; diff --git a/profiling/src/exporter/mod.rs b/profiling/src/exporter/mod.rs index 33dacd4ec..9e2ea54f3 100644 --- a/profiling/src/exporter/mod.rs +++ b/profiling/src/exporter/mod.rs @@ -28,7 +28,7 @@ pub use connector::uds::{socket_path_from_uri, socket_path_to_uri}; #[cfg(windows)] pub use connector::named_pipe::{named_pipe_path_from_uri, named_pipe_path_to_uri}; -use crate::profile::profiled_endpoints::ProfiledEndpointsStats; +use crate::internal::ProfiledEndpointsStats; const DURATION_ZERO: std::time::Duration = std::time::Duration::from_millis(0); diff --git a/profiling/src/profile/profiled_endpoints.rs b/profiling/src/internal/endpoint_stats.rs similarity index 100% rename from profiling/src/profile/profiled_endpoints.rs rename to profiling/src/internal/endpoint_stats.rs diff --git a/profiling/src/profile/internal/endpoints.rs b/profiling/src/internal/endpoints.rs similarity index 92% rename from profiling/src/profile/internal/endpoints.rs rename to profiling/src/internal/endpoints.rs index 14928637f..e80c49c0c 100644 --- a/profiling/src/profile/internal/endpoints.rs +++ b/profiling/src/internal/endpoints.rs @@ -1,7 +1,6 @@ // Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2023-Present Datadog, Inc. use super::*; -use crate::profile::profiled_endpoints::ProfiledEndpointsStats; pub struct Endpoints { pub endpoint_label: StringId, diff --git a/profiling/src/profile/internal/function.rs b/profiling/src/internal/function.rs similarity index 100% rename from profiling/src/profile/internal/function.rs rename to profiling/src/internal/function.rs diff --git a/profiling/src/profile/internal/label.rs b/profiling/src/internal/label.rs similarity index 100% rename from profiling/src/profile/internal/label.rs rename to profiling/src/internal/label.rs diff --git a/profiling/src/profile/internal/location.rs b/profiling/src/internal/location.rs similarity index 100% rename from profiling/src/profile/internal/location.rs rename to profiling/src/internal/location.rs diff --git a/profiling/src/profile/internal/mapping.rs b/profiling/src/internal/mapping.rs similarity index 100% rename from profiling/src/profile/internal/mapping.rs rename to profiling/src/internal/mapping.rs diff --git a/profiling/src/profile/internal/mod.rs b/profiling/src/internal/mod.rs similarity index 85% rename from profiling/src/profile/internal/mod.rs rename to profiling/src/internal/mod.rs index 94e3969cf..ffaa3d5b6 100644 --- a/profiling/src/profile/internal/mod.rs +++ b/profiling/src/internal/mod.rs @@ -1,25 +1,31 @@ // Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2023-Present Datadog, Inc. +mod endpoint_stats; mod endpoints; mod function; mod label; mod location; mod mapping; mod observation; +mod profile; mod sample; mod stack_trace; +mod timestamp; mod upscaling; mod value_type; +pub use endpoint_stats::*; pub use endpoints::*; pub use function::*; pub use label::*; pub use location::*; pub use mapping::*; pub use observation::*; +pub use profile::*; pub use sample::*; pub use stack_trace::*; +pub use timestamp::*; pub use upscaling::*; pub use value_type::*; diff --git a/profiling/src/profile/internal/observation/mod.rs b/profiling/src/internal/observation/mod.rs similarity index 100% rename from profiling/src/profile/internal/observation/mod.rs rename to profiling/src/internal/observation/mod.rs diff --git a/profiling/src/profile/internal/observation/observations.rs b/profiling/src/internal/observation/observations.rs similarity index 99% rename from profiling/src/profile/internal/observation/observations.rs rename to profiling/src/internal/observation/observations.rs index dedc82790..6487ee9ce 100644 --- a/profiling/src/profile/internal/observation/observations.rs +++ b/profiling/src/internal/observation/observations.rs @@ -5,7 +5,7 @@ use super::super::Sample; use super::trimmed_observation::{ObservationLength, TrimmedObservation}; -use crate::profile::Timestamp; +use crate::internal::Timestamp; use std::collections::HashMap; struct NonEmptyObservations { @@ -134,7 +134,7 @@ impl Drop for NonEmptyObservations { mod test { use super::*; use crate::collections::identifiable::*; - use crate::profile::{LabelSetId, StackTraceId}; + use crate::internal::{LabelSetId, StackTraceId}; use std::num::NonZeroI64; #[test] diff --git a/profiling/src/profile/internal/observation/trimmed_observation.rs b/profiling/src/internal/observation/trimmed_observation.rs similarity index 100% rename from profiling/src/profile/internal/observation/trimmed_observation.rs rename to profiling/src/internal/observation/trimmed_observation.rs diff --git a/profiling/src/profile/mod.rs b/profiling/src/internal/profile.rs similarity index 93% rename from profiling/src/profile/mod.rs rename to profiling/src/internal/profile.rs index 5158cde6c..f85797853 100644 --- a/profiling/src/profile/mod.rs +++ b/profiling/src/internal/profile.rs @@ -1,28 +1,17 @@ // Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc. -pub mod api; -pub mod internal; -pub mod pprof; -pub mod profiled_endpoints; - +use self::api::UpscalingInfo; +use super::*; +use crate::api; +use crate::collections::identifiable::*; +use crate::internal::ProfiledEndpointsStats; +use crate::pprof::sliced_proto::*; +use crate::serializer::CompressedProtobufSerializer; use std::borrow::Cow; use std::collections::HashMap; -use std::num::NonZeroI64; use std::time::{Duration, SystemTime}; -use crate::collections::identifiable::*; -use crate::serializer::CompressedProtobufSerializer; -use internal::*; -use pprof::sliced_proto::*; -use profiled_endpoints::ProfiledEndpointsStats; -use prost::EncodeError; - -use self::api::UpscalingInfo; - -pub type Timestamp = NonZeroI64; -pub type TimestampedObservation = (Timestamp, Box<[i64]>); - pub struct Profile { endpoints: Endpoints, functions: FxIndexSet, @@ -40,63 +29,6 @@ pub struct Profile { upscaling_rules: UpscalingRules, } -#[derive(Default)] -pub struct ProfileBuilder<'a> { - period: Option>, - sample_types: Vec>, - start_time: Option, -} - -impl<'a> ProfileBuilder<'a> { - pub const fn new() -> Self { - ProfileBuilder { - period: None, - sample_types: Vec::new(), - start_time: None, - } - } - - pub fn period(mut self, period: Option>) -> Self { - self.period = period; - self - } - - pub fn sample_types(mut self, sample_types: Vec>) -> Self { - self.sample_types = sample_types; - self - } - - pub fn start_time(mut self, start_time: Option) -> Self { - self.start_time = start_time; - self - } - - pub fn build(self) -> Profile { - let mut profile = Profile::new(self.start_time.unwrap_or_else(SystemTime::now)); - - profile.sample_types = self - .sample_types - .iter() - .map(|vt| ValueType { - r#type: profile.intern(vt.r#type), - unit: profile.intern(vt.unit), - }) - .collect(); - - if let Some(period) = self.period { - profile.period = Some(( - period.value, - ValueType { - r#type: profile.intern(period.r#type.r#type), - unit: profile.intern(period.r#type.unit), - }, - )); - }; - - profile - } -} - pub struct EncodedProfile { pub start: SystemTime, pub end: SystemTime, @@ -128,7 +60,11 @@ impl Profile { /// - "local root span id" /// - "trace endpoint" /// All other fields are default. - pub fn new(start_time: SystemTime) -> Self { + pub fn new( + start_time: SystemTime, + sample_types: &[api::ValueType], + period: Option, + ) -> Self { /* Do not use Profile's default() impl here or it will cause a stack * overflow, since that default impl calls this method. */ @@ -156,6 +92,25 @@ impl Profile { profile.endpoints.local_root_span_id_label = profile.intern("local root span id"); profile.endpoints.endpoint_label = profile.intern("trace endpoint"); profile.timestamp_key = profile.intern("end_timestamp_ns"); + + profile.sample_types = sample_types + .iter() + .map(|vt| ValueType { + r#type: profile.intern(vt.r#type), + unit: profile.intern(vt.unit), + }) + .collect(); + + if let Some(period) = period { + profile.period = Some(( + period.value, + ValueType { + r#type: profile.intern(period.r#type.r#type), + unit: profile.intern(period.r#type.unit), + }, + )); + }; + profile } @@ -183,10 +138,6 @@ impl Profile { StringId::from_offset(index) } - pub fn builder<'a>() -> ProfileBuilder<'a> { - ProfileBuilder::new() - } - fn add_stacktrace(&mut self, locations: Vec) -> StackTraceId { self.stack_traces.dedup(StackTrace { locations }) } @@ -333,11 +284,8 @@ impl Profile { value: t.0, }); - let mut profile = ProfileBuilder::new() - .sample_types(sample_types) - .period(period) - .start_time(start_time) - .build(); + let start_time = start_time.unwrap_or_else(SystemTime::now); + let mut profile = Profile::new(start_time, &sample_types, period); std::mem::swap(&mut *self, &mut profile); Ok(profile) @@ -594,7 +542,7 @@ mod api_test { r#type: "samples", unit: "count", }]; - let mut profiles = Profile::builder().sample_types(sample_types).build(); + let mut profiles = Profile::new(SystemTime::now(), &sample_types, None); let expected_id = StringId::from_offset(profiles.interned_strings_count()); @@ -648,7 +596,7 @@ mod api_test { }, ]; - let mut profile = Profile::builder().sample_types(sample_types).build(); + let mut profile = Profile::new(SystemTime::now(), &sample_types, None); assert_eq!(profile.only_for_testing_num_aggregated_samples(), 0); profile @@ -732,7 +680,7 @@ mod api_test { labels, }; - let mut profile = Profile::builder().sample_types(sample_types).build(); + let mut profile = Profile::new(SystemTime::now(), &sample_types, None); assert_eq!(profile.only_for_testing_num_aggregated_samples(), 0); profile @@ -930,7 +878,7 @@ mod api_test { unit: "nanoseconds", }]; - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let id_label = api::Label { key: "local root span id", @@ -961,7 +909,7 @@ mod api_test { }, ]; - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let id_label = api::Label { key: "local root span id", @@ -1075,7 +1023,7 @@ mod api_test { }, ]; - let profile: Profile = Profile::builder().sample_types(sample_types).build(); + let profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let encoded_profile = profile .serialize_into_compressed_pprof(None, None) @@ -1098,7 +1046,7 @@ mod api_test { }, ]; - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let one_endpoint = "my endpoint"; profile.add_endpoint_count(Cow::from(one_endpoint), 1); @@ -1129,7 +1077,7 @@ mod api_test { unit: "nanoseconds", }]; - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let labels = vec![ api::Label { @@ -1168,7 +1116,7 @@ mod api_test { }, ]; - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let id_label = api::Label { key: "my label", @@ -1224,7 +1172,7 @@ mod api_test { fn test_upscaling_by_value_a_zero_value() { let sample_types = create_samples_types(); - let mut profile = Profile::builder().sample_types(sample_types).build(); + let mut profile = Profile::new(SystemTime::now(), &sample_types, None); let sample1 = api::Sample { locations: vec![], @@ -1252,7 +1200,7 @@ mod api_test { fn test_upscaling_by_value_on_one_value() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let sample1 = api::Sample { locations: vec![], @@ -1280,7 +1228,7 @@ mod api_test { fn test_upscaling_by_value_on_one_value_with_poisson() { let sample_types = create_samples_types(); - let mut profile = Profile::builder().sample_types(sample_types).build(); + let mut profile = Profile::new(SystemTime::now(), &sample_types, None); let sample1 = api::Sample { locations: vec![], @@ -1312,7 +1260,7 @@ mod api_test { fn test_upscaling_by_value_on_zero_value_with_poisson() { let sample_types = create_samples_types(); - let mut profile = Profile::builder().sample_types(sample_types).build(); + let mut profile = Profile::new(SystemTime::now(), &sample_types, None); let sample1 = api::Sample { locations: vec![], @@ -1344,7 +1292,7 @@ mod api_test { fn test_cannot_add_a_rule_with_invalid_poisson_info() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let sample1 = api::Sample { locations: vec![], @@ -1391,7 +1339,7 @@ mod api_test { fn test_upscaling_by_value_on_two_values() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let sample1 = api::Sample { locations: vec![], @@ -1448,7 +1396,7 @@ mod api_test { fn test_upscaling_by_value_on_two_value_with_two_rules() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let sample1 = api::Sample { locations: vec![], @@ -1513,7 +1461,7 @@ mod api_test { fn test_no_upscaling_by_label_if_no_match() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let id_label = create_label("my_label", Some("coco")); @@ -1569,7 +1517,7 @@ mod api_test { fn test_upscaling_by_label_on_one_value() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let id_label = create_label("my label", Some("coco")); @@ -1604,7 +1552,7 @@ mod api_test { fn test_upscaling_by_label_on_only_sample_out_of_two() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let id_label = create_label("my label", Some("coco")); @@ -1666,7 +1614,7 @@ mod api_test { fn test_upscaling_by_label_with_two_different_rules_on_two_different_sample() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let id_no_match_label = create_label("another label", Some("do not care")); @@ -1750,7 +1698,7 @@ mod api_test { fn test_upscaling_by_label_on_two_values() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let id_label = create_label("my label", Some("coco")); @@ -1786,7 +1734,7 @@ mod api_test { fn test_upscaling_by_value_and_by_label_different_values() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let id_label = create_label("my label", Some("coco")); @@ -1829,7 +1777,7 @@ mod api_test { fn test_add_same_byvalue_rule_twice() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); // adding same offsets let upscaling_info = UpscalingInfo::Proportional { scale: 2.0 }; @@ -1866,7 +1814,7 @@ mod api_test { fn test_add_two_bylabel_rules_with_overlap_on_values() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); // adding same offsets let mut value_offsets: Vec = vec![0, 2]; @@ -1916,7 +1864,7 @@ mod api_test { fn test_fail_if_bylabel_rule_and_by_value_rule_with_overlap_on_values() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); // adding same offsets let mut value_offsets: Vec = vec![0, 2]; @@ -1970,7 +1918,7 @@ mod api_test { fn test_add_rule_with_offset_out_of_bound() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); // adding same offsets let by_value_offsets: Vec = vec![0, 4]; @@ -1989,7 +1937,7 @@ mod api_test { fn test_add_rule_with_offset_out_of_bound_poisson_function() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); // adding same offsets let by_value_offsets: Vec = vec![0, 4]; @@ -2012,7 +1960,7 @@ mod api_test { fn test_add_rule_with_offset_out_of_bound_poisson_function2() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); // adding same offsets let by_value_offsets: Vec = vec![0, 4]; @@ -2035,7 +1983,7 @@ mod api_test { fn test_add_rule_with_offset_out_of_bound_poisson_function3() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); // adding same offsets let by_value_offsets: Vec = vec![0, 4]; @@ -2058,7 +2006,7 @@ mod api_test { fn test_fails_when_adding_byvalue_rule_collinding_on_offset_with_existing_bylabel_rule() { let sample_types = create_samples_types(); - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile: Profile = Profile::new(SystemTime::now(), &sample_types, None); let id_label = create_label("my label", Some("coco")); @@ -2104,7 +2052,7 @@ mod api_test { }, ]; - let mut profile: Profile = Profile::builder().sample_types(sample_types).build(); + let mut profile = Profile::new(SystemTime::now(), &sample_types, None); let id_label = api::Label { key: "local root span id", diff --git a/profiling/src/profile/internal/sample.rs b/profiling/src/internal/sample.rs similarity index 100% rename from profiling/src/profile/internal/sample.rs rename to profiling/src/internal/sample.rs diff --git a/profiling/src/profile/internal/stack_trace.rs b/profiling/src/internal/stack_trace.rs similarity index 100% rename from profiling/src/profile/internal/stack_trace.rs rename to profiling/src/internal/stack_trace.rs diff --git a/profiling/src/internal/timestamp.rs b/profiling/src/internal/timestamp.rs new file mode 100644 index 000000000..ac1e4828e --- /dev/null +++ b/profiling/src/internal/timestamp.rs @@ -0,0 +1,4 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc. + +pub type Timestamp = std::num::NonZeroI64; diff --git a/profiling/src/profile/internal/upscaling.rs b/profiling/src/internal/upscaling.rs similarity index 98% rename from profiling/src/profile/internal/upscaling.rs rename to profiling/src/internal/upscaling.rs index 2f654d1b3..5af66090d 100644 --- a/profiling/src/profile/internal/upscaling.rs +++ b/profiling/src/internal/upscaling.rs @@ -2,9 +2,9 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2023-Present Datadog, Inc. use super::*; -use crate::profile::api::UpscalingInfo; -use crate::profile::pprof; -use crate::profile::FxIndexMap; +use crate::api::UpscalingInfo; +use crate::collections::identifiable::FxIndexMap; +use crate::pprof; #[derive(Debug)] pub struct UpscalingRule { diff --git a/profiling/src/profile/internal/value_type.rs b/profiling/src/internal/value_type.rs similarity index 100% rename from profiling/src/profile/internal/value_type.rs rename to profiling/src/internal/value_type.rs diff --git a/profiling/src/lib.rs b/profiling/src/lib.rs index d84387c54..444606326 100644 --- a/profiling/src/lib.rs +++ b/profiling/src/lib.rs @@ -1,7 +1,9 @@ // Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc. +pub mod api; pub mod collections; pub mod exporter; -pub mod profile; +pub mod internal; +pub mod pprof; pub mod serializer; diff --git a/profiling/src/profile/pprof/mod.rs b/profiling/src/pprof/mod.rs similarity index 100% rename from profiling/src/profile/pprof/mod.rs rename to profiling/src/pprof/mod.rs diff --git a/profiling/src/profile/pprof/profile.proto b/profiling/src/pprof/profile.proto similarity index 100% rename from profiling/src/profile/pprof/profile.proto rename to profiling/src/pprof/profile.proto diff --git a/profiling/src/profile/pprof/proto.rs b/profiling/src/pprof/proto.rs similarity index 99% rename from profiling/src/profile/pprof/proto.rs rename to profiling/src/pprof/proto.rs index 219244921..4dd124ab9 100644 --- a/profiling/src/profile/pprof/proto.rs +++ b/profiling/src/pprof/proto.rs @@ -1,8 +1,8 @@ // Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc. -use crate::profile::EncodeError; use derivative::Derivative; +use prost::EncodeError; #[derive(Eq, Hash, PartialEq, ::prost::Message)] pub struct Profile { diff --git a/profiling/src/profile/pprof/sliced_proto.rs b/profiling/src/pprof/sliced_proto.rs similarity index 100% rename from profiling/src/profile/pprof/sliced_proto.rs rename to profiling/src/pprof/sliced_proto.rs diff --git a/profiling/src/profile/pprof/test_utils.rs b/profiling/src/pprof/test_utils.rs similarity index 88% rename from profiling/src/profile/pprof/test_utils.rs rename to profiling/src/pprof/test_utils.rs index d1784dddb..86bfbc6be 100644 --- a/profiling/src/profile/pprof/test_utils.rs +++ b/profiling/src/pprof/test_utils.rs @@ -14,7 +14,7 @@ pub fn deserialize_compressed_pprof(encoded: &[u8]) -> anyhow::Result anyhow::Result { +pub fn roundtrip_to_pprof(profile: crate::internal::Profile) -> anyhow::Result { let encoded = profile.serialize_into_compressed_pprof(None, None)?; deserialize_compressed_pprof(&encoded.buffer) } diff --git a/profiling/tests/wordpress.rs b/profiling/tests/wordpress.rs index 14e6716b2..5de4b2c67 100644 --- a/profiling/tests/wordpress.rs +++ b/profiling/tests/wordpress.rs @@ -1,7 +1,8 @@ // Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc. -use datadog_profiling::profile::*; +use datadog_profiling::api; +use datadog_profiling::pprof; use lz4_flex::frame::FrameDecoder; use prost::Message; use std::fs::File; From 0407e5c5f0fdae2623ea01d1a60df5037de84e6f Mon Sep 17 00:00:00 2001 From: Bob Weinand Date: Wed, 27 Sep 2023 16:55:55 +0200 Subject: [PATCH 3/6] Fix version in sidecar ffi for dependencies and integrations (#265) Signed-off-by: Bob Weinand --- sidecar-ffi/src/unix.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/sidecar-ffi/src/unix.rs b/sidecar-ffi/src/unix.rs index f60e62410..587cda271 100644 --- a/sidecar-ffi/src/unix.rs +++ b/sidecar-ffi/src/unix.rs @@ -285,9 +285,8 @@ pub unsafe extern "C" fn ddog_sidecar_telemetry_addDependency( dependency_name: ffi::CharSlice, dependency_version: ffi::CharSlice, ) -> MaybeError { - let version = dependency_version - .is_empty() - .then(|| dependency_version.to_utf8_lossy().into_owned()); + let version = + (!dependency_version.is_empty()).then(|| dependency_version.to_utf8_lossy().into_owned()); let dependency = TelemetryActions::AddDependecy(Dependency { name: dependency_name.to_utf8_lossy().into_owned(), @@ -314,9 +313,8 @@ pub unsafe extern "C" fn ddog_sidecar_telemetry_addIntegration( integration_version: ffi::CharSlice, integration_enabled: bool, ) -> MaybeError { - let version = integration_version - .is_empty() - .then(|| integration_version.to_utf8_lossy().into_owned()); + let version = + (!integration_version.is_empty()).then(|| integration_version.to_utf8_lossy().into_owned()); let integration = TelemetryActions::AddIntegration(Integration { name: integration_name.to_utf8_lossy().into_owned(), From ad54c79a61e2ba3b5c48c56531a30e5622aab64b Mon Sep 17 00:00:00 2001 From: Daniel Schwartz-Narbonne Date: Wed, 27 Sep 2023 11:28:35 -0400 Subject: [PATCH 4/6] Refactor: reorganize functions in internal/profile to clarify the public api (#264) --- profiling-ffi/src/profiles.rs | 2 +- profiling-replayer/src/main.rs | 2 +- profiling/examples/profiles.rs | 2 +- profiling/src/internal/profile.rs | 489 +++++++++++++++--------------- 4 files changed, 253 insertions(+), 242 deletions(-) diff --git a/profiling-ffi/src/profiles.rs b/profiling-ffi/src/profiles.rs index 9994bf6ea..5f0abeaff 100644 --- a/profiling-ffi/src/profiles.rs +++ b/profiling-ffi/src/profiles.rs @@ -423,7 +423,7 @@ unsafe fn ddog_prof_profile_add_impl( ) -> anyhow::Result<()> { let profile = profile_ptr_to_inner(profile_ptr)?; - match sample.try_into().map(|s| profile.add(s, timestamp)) { + match sample.try_into().map(|s| profile.add_sample(s, timestamp)) { Ok(r) => match r { Ok(_) => Ok(()), Err(err) => Err(err), diff --git a/profiling-replayer/src/main.rs b/profiling-replayer/src/main.rs index 7eae2ded3..aa80342f2 100644 --- a/profiling-replayer/src/main.rs +++ b/profiling-replayer/src/main.rs @@ -183,7 +183,7 @@ fn main() -> anyhow::Result<()> { let before = Instant::now(); for (timestamp, sample) in samples { - outprof.add(sample, timestamp)?; + outprof.add_sample(sample, timestamp)?; } let duration = before.elapsed(); diff --git a/profiling/examples/profiles.rs b/profiling/examples/profiles.rs index 2b9e9c04c..0c23bd395 100644 --- a/profiling/examples/profiles.rs +++ b/profiling/examples/profiles.rs @@ -59,7 +59,7 @@ fn main() { // Intentionally use the current time. let mut profile = Profile::new(SystemTime::now(), &sample_types, Some(period)); - match profile.add(sample, None) { + match profile.add_sample(sample, None) { Ok(_) => {} Err(_) => exit(1), } diff --git a/profiling/src/internal/profile.rs b/profiling/src/internal/profile.rs index f85797853..bfd066b18 100644 --- a/profiling/src/internal/profile.rs +++ b/profiling/src/internal/profile.rs @@ -36,24 +36,94 @@ pub struct EncodedProfile { pub endpoints_stats: ProfiledEndpointsStats, } -// For testing and debugging purposes +/// Public API impl Profile { - pub fn only_for_testing_num_aggregated_samples(&self) -> usize { - self.observations + /// Add the endpoint data to the endpoint mappings. + /// The `endpoint` string will be interned. + pub fn add_endpoint(&mut self, local_root_span_id: u64, endpoint: Cow) { + let interned_endpoint = self.intern(endpoint.as_ref()); + + self.endpoints + .mappings + .insert(local_root_span_id, interned_endpoint); + } + + pub fn add_endpoint_count(&mut self, endpoint: Cow, value: i64) { + self.endpoints + .stats + .add_endpoint_count(endpoint.into_owned(), value); + } + + pub fn add_sample( + &mut self, + sample: api::Sample, + timestamp: Option, + ) -> anyhow::Result<()> { + anyhow::ensure!( + sample.values.len() == self.sample_types.len(), + "expected {} sample types, but sample had {} sample types", + self.sample_types.len(), + sample.values.len(), + ); + + self.validate_sample_labels(&sample)?; + let labels: Vec<_> = sample + .labels .iter() - .filter(|(_, ts, _)| ts.is_none()) - .count() + .map(|label| { + let key = self.intern(label.key); + let internal_label = if let Some(s) = label.str { + let str = self.intern(s); + Label::str(key, str) + } else { + let num = label.num; + let num_unit = label.num_unit.map(|s| self.intern(s)); + Label::num(key, num, num_unit) + }; + + self.labels.dedup(internal_label) + }) + .collect(); + let labels = self.label_sets.dedup(LabelSet::new(labels)); + + let locations = sample + .locations + .iter() + .map(|l| self.add_location(l)) + .collect(); + + let stacktrace = self.add_stacktrace(locations); + self.observations + .add(Sample::new(labels, stacktrace), timestamp, sample.values); + Ok(()) } - pub fn only_for_testing_num_timestamped_samples(&self) -> usize { - use std::collections::HashSet; - let sample_set: HashSet = - HashSet::from_iter(self.observations.iter().filter_map(|(_, ts, _)| ts)); - sample_set.len() + pub fn add_upscaling_rule( + &mut self, + offset_values: &[usize], + label_name: &str, + label_value: &str, + upscaling_info: UpscalingInfo, + ) -> anyhow::Result<()> { + let label_name_id = self.intern(label_name); + let label_value_id = self.intern(label_value); + self.upscaling_rules.add( + offset_values, + (label_name, label_name_id), + (label_value, label_value_id), + upscaling_info, + self.sample_types.len(), + )?; + + Ok(()) + } + + pub fn get_string(&self, id: StringId) -> &str { + self.strings + .get_index(id.to_offset()) + .expect("StringId to have a valid interned index") } -} -impl Profile { /// Creates a profile with `start_time`. /// Initializes the string table to hold: /// - "" (the empty string) @@ -114,156 +184,6 @@ impl Profile { profile } - #[cfg(test)] - fn interned_strings_count(&self) -> usize { - self.strings.len() - } - - /// Interns the `str` as a string, returning the id in the string table. - /// The empty string is guaranteed to have an id of [StringId::ZERO]. - fn intern(&mut self, item: &str) -> StringId { - // For performance, delay converting the [&str] to a [String] until - // after it has been determined to not exist in the set. This avoids - // temporary allocations. - let index = match self.strings.get_index_of(item) { - Some(index) => index, - None => { - let (index, _inserted) = self.strings.insert_full(item.into()); - // This wouldn't make any sense; the item couldn't be found so - // we try to insert it, but suddenly it exists now? - debug_assert!(_inserted); - index - } - }; - StringId::from_offset(index) - } - - fn add_stacktrace(&mut self, locations: Vec) -> StackTraceId { - self.stack_traces.dedup(StackTrace { locations }) - } - - fn get_stacktrace(&self, st: StackTraceId) -> &StackTrace { - self.stack_traces - .get_index(st.to_raw_id()) - .expect("StackTraceId {st} to exist in profile") - } - - fn add_function(&mut self, function: &api::Function) -> FunctionId { - let name = self.intern(function.name); - let system_name = self.intern(function.system_name); - let filename = self.intern(function.filename); - - let start_line = function.start_line; - self.functions.dedup(Function { - name, - system_name, - filename, - start_line, - }) - } - - fn add_location(&mut self, location: &api::Location) -> LocationId { - let mapping_id = self.add_mapping(&location.mapping); - let function_id = self.add_function(&location.function); - self.locations.dedup(Location { - mapping_id, - function_id, - address: location.address, - line: location.line, - }) - } - - fn add_mapping(&mut self, mapping: &api::Mapping) -> MappingId { - let filename = self.intern(mapping.filename); - let build_id = self.intern(mapping.build_id); - - self.mappings.dedup(Mapping { - memory_start: mapping.memory_start, - memory_limit: mapping.memory_limit, - file_offset: mapping.file_offset, - filename, - build_id, - }) - } - - pub fn add(&mut self, sample: api::Sample, timestamp: Option) -> anyhow::Result<()> { - anyhow::ensure!( - sample.values.len() == self.sample_types.len(), - "expected {} sample types, but sample had {} sample types", - self.sample_types.len(), - sample.values.len(), - ); - - self.validate_sample_labels(&sample)?; - let labels: Vec<_> = sample - .labels - .iter() - .map(|label| { - let key = self.intern(label.key); - let internal_label = if let Some(s) = label.str { - let str = self.intern(s); - Label::str(key, str) - } else { - let num = label.num; - let num_unit = label.num_unit.map(|s| self.intern(s)); - Label::num(key, num, num_unit) - }; - - self.labels.dedup(internal_label) - }) - .collect(); - let labels = self.label_sets.dedup(LabelSet::new(labels)); - - let locations = sample - .locations - .iter() - .map(|l| self.add_location(l)) - .collect(); - - let stacktrace = self.add_stacktrace(locations); - self.observations - .add(Sample::new(labels, stacktrace), timestamp, sample.values); - Ok(()) - } - - /// Validates labels - fn validate_sample_labels(&mut self, sample: &api::Sample) -> anyhow::Result<()> { - let mut seen: HashMap<&str, &api::Label> = HashMap::new(); - - for label in sample.labels.iter() { - if let Some(duplicate) = seen.insert(label.key, label) { - anyhow::bail!("Duplicate label on sample: {:?} {:?}", duplicate, label); - } - - if label.key == "local root span id" { - anyhow::ensure!( - label.str.is_none() && label.num != 0, - "Invalid \"local root span id\" label: {:?}", - label - ); - } - - anyhow::ensure!( - label.key != "end_timestamp_ns", - "Timestamp should not be passed as a label {:?}", - label - ); - } - Ok(()) - } - - fn extract_api_sample_types(&self) -> anyhow::Result> { - let sample_types = self - .sample_types - .iter() - .map(|sample_type| api::ValueType { - r#type: self.get_string(sample_type.r#type), - unit: self.get_string(sample_type.unit), - }) - .collect(); - Ok(sample_types) - } - /// Resets all data except the sample types and period. /// Returns the previous Profile on success. pub fn reset_and_return_previous( @@ -291,42 +211,6 @@ impl Profile { Ok(profile) } - /// Add the endpoint data to the endpoint mappings. - /// The `endpoint` string will be interned. - pub fn add_endpoint(&mut self, local_root_span_id: u64, endpoint: Cow) { - let interned_endpoint = self.intern(endpoint.as_ref()); - - self.endpoints - .mappings - .insert(local_root_span_id, interned_endpoint); - } - - pub fn add_endpoint_count(&mut self, endpoint: Cow, value: i64) { - self.endpoints - .stats - .add_endpoint_count(endpoint.into_owned(), value); - } - - pub fn add_upscaling_rule( - &mut self, - offset_values: &[usize], - label_name: &str, - label_value: &str, - upscaling_info: UpscalingInfo, - ) -> anyhow::Result<()> { - let label_name_id = self.intern(label_name); - let label_value_id = self.intern(label_value); - self.upscaling_rules.add( - offset_values, - (label_name, label_name_id), - (label_value, label_value_id), - upscaling_info, - self.sample_types.len(), - )?; - - Ok(()) - } - /// Serialize the aggregated profile, adding the end time and duration. /// # Arguments /// * `end_time` - Optional end time of the profile. Passing None will use the current time. @@ -440,23 +324,62 @@ impl Profile { endpoints_stats, }) } +} - pub fn get_label(&self, id: LabelId) -> &Label { - self.labels - .get_index(id.to_offset()) - .expect("LabelId to have a valid interned index") +/// Private helper functions +impl Profile { + fn add_function(&mut self, function: &api::Function) -> FunctionId { + let name = self.intern(function.name); + let system_name = self.intern(function.system_name); + let filename = self.intern(function.filename); + + let start_line = function.start_line; + self.functions.dedup(Function { + name, + system_name, + filename, + start_line, + }) } - pub fn get_label_set(&self, id: LabelSetId) -> &LabelSet { - self.label_sets - .get_index(id.to_offset()) - .expect("LabelSetId to have a valid interned index") + fn add_location(&mut self, location: &api::Location) -> LocationId { + let mapping_id = self.add_mapping(&location.mapping); + let function_id = self.add_function(&location.function); + self.locations.dedup(Location { + mapping_id, + function_id, + address: location.address, + line: location.line, + }) } - pub fn get_string(&self, id: StringId) -> &str { - self.strings - .get_index(id.to_offset()) - .expect("StringId to have a valid interned index") + fn add_mapping(&mut self, mapping: &api::Mapping) -> MappingId { + let filename = self.intern(mapping.filename); + let build_id = self.intern(mapping.build_id); + + self.mappings.dedup(Mapping { + memory_start: mapping.memory_start, + memory_limit: mapping.memory_limit, + file_offset: mapping.file_offset, + filename, + build_id, + }) + } + + fn add_stacktrace(&mut self, locations: Vec) -> StackTraceId { + self.stack_traces.dedup(StackTrace { locations }) + } + + fn extract_api_sample_types(&self) -> anyhow::Result> { + let sample_types = self + .sample_types + .iter() + .map(|sample_type| api::ValueType { + r#type: self.get_string(sample_type.r#type), + unit: self.get_string(sample_type.unit), + }) + .collect(); + Ok(sample_types) } /// Fetches the endpoint information for the label. There may be errors, @@ -510,6 +433,43 @@ impl Profile { } } + fn get_label(&self, id: LabelId) -> &Label { + self.labels + .get_index(id.to_offset()) + .expect("LabelId to have a valid interned index") + } + + fn get_label_set(&self, id: LabelSetId) -> &LabelSet { + self.label_sets + .get_index(id.to_offset()) + .expect("LabelSetId to have a valid interned index") + } + + fn get_stacktrace(&self, st: StackTraceId) -> &StackTrace { + self.stack_traces + .get_index(st.to_raw_id()) + .expect("StackTraceId {st} to exist in profile") + } + + /// Interns the `str` as a string, returning the id in the string table. + /// The empty string is guaranteed to have an id of [StringId::ZERO]. + fn intern(&mut self, item: &str) -> StringId { + // For performance, delay converting the [&str] to a [String] until + // after it has been determined to not exist in the set. This avoids + // temporary allocations. + let index = match self.strings.get_index_of(item) { + Some(index) => index, + None => { + let (index, _inserted) = self.strings.insert_full(item.into()); + // This wouldn't make any sense; the item couldn't be found so + // we try to insert it, but suddenly it exists now? + debug_assert!(_inserted); + index + } + }; + StringId::from_offset(index) + } + fn translate_and_enrich_sample_labels( &self, sample: Sample, @@ -528,6 +488,57 @@ impl Profile { Ok(labels) } + + /// Validates labels + fn validate_sample_labels(&mut self, sample: &api::Sample) -> anyhow::Result<()> { + let mut seen: HashMap<&str, &api::Label> = HashMap::new(); + + for label in sample.labels.iter() { + if let Some(duplicate) = seen.insert(label.key, label) { + anyhow::bail!("Duplicate label on sample: {:?} {:?}", duplicate, label); + } + + if label.key == "local root span id" { + anyhow::ensure!( + label.str.is_none() && label.num != 0, + "Invalid \"local root span id\" label: {:?}", + label + ); + } + + anyhow::ensure!( + label.key != "end_timestamp_ns", + "Timestamp should not be passed as a label {:?}", + label + ); + } + Ok(()) + } +} + +/// For testing and debugging purposes +impl Profile { + #[cfg(test)] + fn interned_strings_count(&self) -> usize { + self.strings.len() + } + + // Ideally, these would be [cgf(test)]. But its used in other module's test + // code, which would break if we did so. We could try to do something with + // a test "feature", but this naming scheme is sufficient for now. + pub fn only_for_testing_num_aggregated_samples(&self) -> usize { + self.observations + .iter() + .filter(|(_, ts, _)| ts.is_none()) + .count() + } + + pub fn only_for_testing_num_timestamped_samples(&self) -> usize { + use std::collections::HashSet; + let sample_set: HashSet = + HashSet::from_iter(self.observations.iter().filter_map(|(_, ts, _)| ts)); + sample_set.len() + } } #[cfg(test)] @@ -600,7 +611,7 @@ mod api_test { assert_eq!(profile.only_for_testing_num_aggregated_samples(), 0); profile - .add( + .add_sample( api::Sample { locations, values: vec![1, 10000], @@ -684,18 +695,18 @@ mod api_test { assert_eq!(profile.only_for_testing_num_aggregated_samples(), 0); profile - .add(main_sample, None) + .add_sample(main_sample, None) .expect("profile to not be full"); assert_eq!(profile.only_for_testing_num_aggregated_samples(), 1); profile - .add(test_sample, None) + .add_sample(test_sample, None) .expect("profile to not be full"); assert_eq!(profile.only_for_testing_num_aggregated_samples(), 2); assert_eq!(profile.only_for_testing_num_timestamped_samples(), 0); profile - .add(timestamp_sample, Timestamp::new(42)) + .add_sample(timestamp_sample, Timestamp::new(42)) .expect("profile to not be full"); assert_eq!(profile.only_for_testing_num_timestamped_samples(), 1); profile @@ -893,7 +904,7 @@ mod api_test { labels: vec![id_label], }; - assert!(profile.add(sample, None).is_err()); + assert!(profile.add_sample(sample, None).is_err()); } #[test] @@ -944,9 +955,9 @@ mod api_test { labels: vec![id2_label, other_label], }; - profile.add(sample1, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); - profile.add(sample2, None).expect("add to success"); + profile.add_sample(sample2, None).expect("add to success"); profile.add_endpoint(10, Cow::from("my endpoint")); @@ -1100,7 +1111,7 @@ mod api_test { labels, }; - profile.add(sample, None).unwrap_err(); + profile.add_sample(sample, None).unwrap_err(); } #[test] @@ -1131,7 +1142,7 @@ mod api_test { labels: vec![id_label], }; - profile.add(sample1, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); let serialized_profile = pprof::roundtrip_to_pprof(profile).unwrap(); @@ -1180,7 +1191,7 @@ mod api_test { labels: vec![], }; - profile.add(sample1, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); let upscaling_info = UpscalingInfo::Proportional { scale: 2.0 }; let values_offset = vec![0]; @@ -1208,7 +1219,7 @@ mod api_test { labels: vec![], }; - profile.add(sample1, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); let upscaling_info = UpscalingInfo::Proportional { scale: 2.7 }; let values_offset = vec![0]; @@ -1236,7 +1247,7 @@ mod api_test { labels: vec![], }; - profile.add(sample1, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); let upscaling_info = UpscalingInfo::Poisson { sum_value_offset: 1, @@ -1268,7 +1279,7 @@ mod api_test { labels: vec![], }; - profile.add(sample1, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); let upscaling_info = UpscalingInfo::Poisson { sum_value_offset: 1, @@ -1300,7 +1311,7 @@ mod api_test { labels: vec![], }; - profile.add(sample1, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); // invalid sampling_distance vaue let upscaling_info = UpscalingInfo::Poisson { @@ -1370,8 +1381,8 @@ mod api_test { labels: vec![], }; - profile.add(sample1, None).expect("add to success"); - profile.add(sample2, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); + profile.add_sample(sample2, None).expect("add to success"); // upscale the first value and the last one let values_offset: Vec = vec![0, 2]; @@ -1426,8 +1437,8 @@ mod api_test { labels: vec![], }; - profile.add(sample1, None).expect("add to success"); - profile.add(sample2, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); + profile.add_sample(sample2, None).expect("add to success"); let mut values_offset: Vec = vec![0]; @@ -1471,7 +1482,7 @@ mod api_test { labels: vec![id_label], }; - profile.add(sample1, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); let values_offset: Vec = vec![0]; @@ -1527,7 +1538,7 @@ mod api_test { labels: vec![id_label], }; - profile.add(sample1, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); let upscaling_info = UpscalingInfo::Proportional { scale: 2.0 }; let values_offset: Vec = vec![0]; @@ -1584,8 +1595,8 @@ mod api_test { labels: vec![], }; - profile.add(sample1, None).expect("add to success"); - profile.add(sample2, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); + profile.add_sample(sample2, None).expect("add to success"); let upscaling_info = UpscalingInfo::Proportional { scale: 2.0 }; let values_offset: Vec = vec![0]; @@ -1655,8 +1666,8 @@ mod api_test { labels: vec![id_no_match_label, id_label2], }; - profile.add(sample1, None).expect("add to success"); - profile.add(sample2, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); + profile.add_sample(sample2, None).expect("add to success"); // add rule for the first sample on the 1st value let upscaling_info = UpscalingInfo::Proportional { scale: 2.0 }; @@ -1708,7 +1719,7 @@ mod api_test { labels: vec![id_label], }; - profile.add(sample1, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); // upscale samples and wall-time values let values_offset: Vec = vec![0, 1]; @@ -1744,7 +1755,7 @@ mod api_test { labels: vec![id_label], }; - profile.add(sample1, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); let upscaling_info = UpscalingInfo::Proportional { scale: 2.0 }; let mut value_offsets: Vec = vec![0]; @@ -2016,7 +2027,7 @@ mod api_test { labels: vec![id_label], }; - profile.add(sample1, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); let mut value_offsets: Vec = vec![0, 1]; // Add by-label rule first @@ -2084,8 +2095,8 @@ mod api_test { labels: vec![id2_label], }; - profile.add(sample1, None).expect("add to success"); - profile.add(sample2, None).expect("add to success"); + profile.add_sample(sample1, None).expect("add to success"); + profile.add_sample(sample2, None).expect("add to success"); profile.add_endpoint(10, Cow::from("endpoint 10")); profile.add_endpoint(large_span_id, Cow::from("large endpoint")); From 83b6fbb7123bcd8c269290bcdc4e3bd6c4247c4c Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Tue, 3 Oct 2023 15:25:18 +0100 Subject: [PATCH 5/6] Bump libdatadog version to 5.0.0 in preparation for release **What does this PR do?** This PR bumps the libdatadog version to 5.0.0. This PR looks a bit different from previous PRs to bump libdatadog version [(here's the 4.0.0 PR as an example)](https://github.com/DataDog/libdatadog/pull/236) because in [this PR](https://github.com/DataDog/libdatadog/pull/247) we've centralized the version on the single `Cargo.toml` file. Furthermore, we're going from 4.0.0 to 5.0.0 because there were a number of backwards-incompatible changes to the profiling APIs. **Motivation:** Release libdatadog 5.5.0. **Additional Notes:** If I haven't missed anything, the backwards incompatible API changes were the following: * The value of the `end_timestamp_ns` label is now provided as a regular argument to `ddog_prof_Profile_add` * The libdatadog 5 serializer outputs compressed pprof files * The exporter has a new API that takes two lists, a list of files to compress and a list of files to assume are compressed when exporting * The libdatadog 5 serializer now resets profiles as part of serializing them * The `ddog_prof_Profile_new` now returns a result structure **How to test the change?** I've tested the libdatadog 5 releases using the Ruby profiler, see https://github.com/DataDog/dd-trace-rb/pull/3169 for my draft PR. --- Cargo.lock | 24 ++++++++++++------------ Cargo.toml | 2 +- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 30210894f..34ae3cae0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -595,7 +595,7 @@ dependencies = [ [[package]] name = "datadog-profiling" -version = "4.0.0" +version = "5.0.0" dependencies = [ "anyhow", "bitmaps", @@ -626,7 +626,7 @@ dependencies = [ [[package]] name = "datadog-profiling-ffi" -version = "4.0.0" +version = "5.0.0" dependencies = [ "anyhow", "chrono", @@ -642,7 +642,7 @@ dependencies = [ [[package]] name = "datadog-profiling-replayer" -version = "4.0.0" +version = "5.0.0" dependencies = [ "anyhow", "clap", @@ -754,7 +754,7 @@ dependencies = [ [[package]] name = "datadog-trace-normalization" -version = "4.0.0" +version = "5.0.0" dependencies = [ "anyhow", "datadog-trace-protobuf", @@ -764,7 +764,7 @@ dependencies = [ [[package]] name = "datadog-trace-obfuscation" -version = "4.0.0" +version = "5.0.0" dependencies = [ "anyhow", "criterion", @@ -782,7 +782,7 @@ dependencies = [ [[package]] name = "datadog-trace-protobuf" -version = "4.0.0" +version = "5.0.0" dependencies = [ "prost", "prost-build", @@ -793,7 +793,7 @@ dependencies = [ [[package]] name = "datadog-trace-utils" -version = "4.0.0" +version = "5.0.0" dependencies = [ "anyhow", "datadog-trace-normalization", @@ -813,7 +813,7 @@ dependencies = [ [[package]] name = "ddcommon" -version = "4.0.0" +version = "5.0.0" dependencies = [ "anyhow", "futures", @@ -838,7 +838,7 @@ dependencies = [ [[package]] name = "ddcommon-ffi" -version = "4.0.0" +version = "5.0.0" dependencies = [ "anyhow", "ddcommon", @@ -847,7 +847,7 @@ dependencies = [ [[package]] name = "ddtelemetry" -version = "4.0.0" +version = "5.0.0" dependencies = [ "anyhow", "ddcommon", @@ -871,7 +871,7 @@ dependencies = [ [[package]] name = "ddtelemetry-ffi" -version = "4.0.0" +version = "5.0.0" dependencies = [ "ddcommon", "ddcommon-ffi", @@ -2925,7 +2925,7 @@ dependencies = [ [[package]] name = "tools" -version = "4.0.0" +version = "5.0.0" dependencies = [ "lazy_static", "regex", diff --git a/Cargo.toml b/Cargo.toml index 7b3eb4553..2a5f8b292 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,7 @@ resolver = "2" [workspace.package] rust-version = "1.69" edition = "2021" -version = "4.0.0" +version = "5.0.0" license = "Apache-2.0" [profile.dev] From 426db24f5117f928bcd1926e46867c53dd0ba484 Mon Sep 17 00:00:00 2001 From: Ivo Anjo Date: Wed, 4 Oct 2023 11:05:12 +0100 Subject: [PATCH 6/6] [PROF-8289] Package libdatadog v5.0.0 for Ruby **What does this PR do?** This PR includes the changes documented in the "Releasing a new version to rubygems.org" part of the README: (It's also exactly the same as [the v4.0.0 release PR](https://github.com/DataDog/libdatadog/pull/238)). **Motivation:** Enable Ruby to use libdatadog v5.0.0. **Additional Notes:** N/A **How to test the change?** I've tested this release locally against the changes in https://github.com/DataDog/dd-trace-rb/pull/3169 . As a reminder, new libdatadog releases don't get automatically picked up by dd-trace-rb, so the PR that bumps the Ruby profiler will also test this release against all supported Ruby versions. --- ruby/Rakefile | 10 +++++----- ruby/lib/libdatadog/version.rb | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ruby/Rakefile b/ruby/Rakefile index 2c5a15ed0..12bc373cd 100644 --- a/ruby/Rakefile +++ b/ruby/Rakefile @@ -11,7 +11,7 @@ require "rubygems/package" RSpec::Core::RakeTask.new(:spec) -LIB_VERSION_TO_PACKAGE = "4.0.0" +LIB_VERSION_TO_PACKAGE = "5.0.0" unless LIB_VERSION_TO_PACKAGE.start_with?(Libdatadog::LIB_VERSION) raise "`LIB_VERSION_TO_PACKAGE` setting in (#{LIB_VERSION_TO_PACKAGE}) does not match " \ "`LIB_VERSION` setting in (#{Libdatadog::LIB_VERSION})" @@ -20,22 +20,22 @@ end LIB_GITHUB_RELEASES = [ { file: "libdatadog-aarch64-alpine-linux-musl.tar.gz", - sha256: "8819ede8a8dbbc133e014426ac2b151a31b977142a228afec3759e7a66fc2a4f", + sha256: "f5fb14372b8d6018f4759eb81447dfec0d3393e8e4e44fe890c42045563b5de4", ruby_platform: "aarch64-linux-musl" }, { file: "libdatadog-aarch64-unknown-linux-gnu.tar.gz", - sha256: "dff1c1b87c9cc304aa8f009421117b3822190055ba34cd267c8f4500c46637c0", + sha256: "2b3d1c5c3965ab4a9436aff4e101814eddaa59b59cb984ce6ebda45613aadbc3", ruby_platform: "aarch64-linux" }, { file: "libdatadog-x86_64-alpine-linux-musl.tar.gz", - sha256: "e65540dc0c21fbc06ae884622f17c73c52e475613c0628f9a9630624d8bb01b2", + sha256: "060482ff1c34940cf7fad1dc841693602e04e4fa54ac9e9f08cb688efcbab137", ruby_platform: "x86_64-linux-musl" }, { file: "libdatadog-x86_64-unknown-linux-gnu.tar.gz", - sha256: "a987f8dd6c1aae92fc33613ae3b11473f701ef0f1a858c3a9b0673b83d2c2a90", + sha256: "11c09440271dd4374b8fca8f0faa66c43a5e057aae05902543beb1e6cb382e52", ruby_platform: "x86_64-linux" } ] diff --git a/ruby/lib/libdatadog/version.rb b/ruby/lib/libdatadog/version.rb index 58d5d3ab1..cf5e0ff0b 100644 --- a/ruby/lib/libdatadog/version.rb +++ b/ruby/lib/libdatadog/version.rb @@ -2,7 +2,7 @@ module Libdatadog # Current libdatadog version - LIB_VERSION = "4.0.0" + LIB_VERSION = "5.0.0" GEM_MAJOR_VERSION = "1" GEM_MINOR_VERSION = "0"