Skip to content

feat: [SVLS-5992] support DD_URL and DD_DD_URL for metrics intake #517

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.

Sign up for GitHub

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

Closed
apiarian-datadog wants to merge 2 commits into from
Closed

feat: [SVLS-5992] support DD_URL and DD_DD_URL for metrics intake #517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b35a023
feat: [SVLS-5992] support DD_URL and DD_DD_URL for metrics intake
apiarian-datadog Jan 16, 2025
ccba5c7
chore: [SVLS-5992] push config parsing upwards
apiarian-datadog Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 36 additions & 20 deletions bottlecap/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions bottlecap/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ async-trait = { version = "0.1", default-features = false }
chrono = { version = "0.4", features = ["serde", "std", "now"], default-features = false }
datadog-protos = { version = "0.1.0", default-features = false, git = "https://github.com/DataDog/saluki/" }
ddsketch-agent = { version = "0.1.0", default-features = false, git = "https://github.com/DataDog/saluki/" }
ddcommon = { git = "https://github.com/DataDog/libdatadog", rev = "dbc8869c6de4ccd78a770524d250420f397fc7b3" }
datadog-trace-protobuf = { git = "https://github.com/DataDog/libdatadog", rev = "dbc8869c6de4ccd78a770524d250420f397fc7b3" }
datadog-trace-utils = { git = "https://github.com/DataDog/libdatadog", rev = "dbc8869c6de4ccd78a770524d250420f397fc7b3" }
datadog-trace-mini-agent = { git = "https://github.com/DataDog/libdatadog", rev = "dbc8869c6de4ccd78a770524d250420f397fc7b3" }
datadog-trace-normalization = { git = "https://github.com/DataDog/libdatadog", rev = "dbc8869c6de4ccd78a770524d250420f397fc7b3" }
datadog-trace-obfuscation = { git = "https://github.com/DataDog/libdatadog", rev = "dbc8869c6de4ccd78a770524d250420f397fc7b3" }
dogstatsd = { git = "https://github.com/DataDog/libdatadog", rev = "dbc8869c6de4ccd78a770524d250420f397fc7b3" }
ddcommon = { git = "https://github.com/DataDog/libdatadog", rev = "6e2d4462873e35e49a2c9befde958d96c4ed9de4" }
datadog-trace-protobuf = { git = "https://github.com/DataDog/libdatadog", rev = "6e2d4462873e35e49a2c9befde958d96c4ed9de4" }
datadog-trace-utils = { git = "https://github.com/DataDog/libdatadog", rev = "6e2d4462873e35e49a2c9befde958d96c4ed9de4" }
datadog-trace-mini-agent = { git = "https://github.com/DataDog/libdatadog", rev = "6e2d4462873e35e49a2c9befde958d96c4ed9de4" }
datadog-trace-normalization = { git = "https://github.com/DataDog/libdatadog", rev = "6e2d4462873e35e49a2c9befde958d96c4ed9de4" }
datadog-trace-obfuscation = { git = "https://github.com/DataDog/libdatadog", rev = "6e2d4462873e35e49a2c9befde958d96c4ed9de4" }
dogstatsd = { git = "https://github.com/DataDog/libdatadog", rev = "6e2d4462873e35e49a2c9befde958d96c4ed9de4" }
figment = { version = "0.10", default-features = false, features = ["yaml", "env"] }
hyper = { version = "0.14", default-features = false, features = ["server"] }
lazy_static = { version = "1.5", default-features = false }
Expand Down
64 changes: 55 additions & 9 deletions bottlecap/src/bin/bottlecap/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ use decrypt::resolve_secrets;
use dogstatsd::{
aggregator::Aggregator as MetricsAggregator,
constants::CONTEXTS,
datadog::{DdDdUrl, DdUrl, MetricsIntakeUrlPrefix, MetricsIntakeUrlPrefixOverride, Site},
dogstatsd::{DogStatsD, DogStatsDConfig},
flusher::{build_fqdn_metrics, Flusher as MetricsFlusher},
flusher::{Flusher as MetricsFlusher, FlusherConfig as MetricsFlusherConfig},
metric::{SortedTags, EMPTY_TAGS},
};
use reqwest::Client;
Expand Down Expand Up @@ -163,6 +164,41 @@ fn build_function_arn(account_id: &str, region: &str, function_name: &str) -> St
async fn main() -> Result<()> {
let (aws_config, config) = load_configs();

let site = Site::new(config.site.clone()).map_err(|e| {
Copy link
Contributor Author

@apiarian-datadog apiarian-datadog Jan 17, 2025

Choose a reason for hiding this comment

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

@duncanista, @alexgallotta : how would y'all want me to handle this config parsing block? should i push it into load_configs() and change that to return aws_config, config, and metrics_intake_url_prefix? or something else?

Error::new(
std::io::ErrorKind::InvalidData,
format!("Failed to parse DD_SITE: {e:?}"),
)
})?;
let dd_url = match config.url.clone() {
Some(dd_url) => Some(DdUrl::new(dd_url).map_err(|e| {
Error::new(
std::io::ErrorKind::InvalidData,
format!("Failed to parse DD_URL: {e:?}"),
)
})?),
None => None,
};
let dd_dd_url = match config.dd_url.clone() {
Some(dd_dd_url) => Some(DdDdUrl::new(dd_dd_url).map_err(|e| {
Error::new(
std::io::ErrorKind::InvalidData,
format!("Failed to parse DD_DD_URL: {e:?}"),
)
})?),
None => None,
};
let metrics_intake_url_prefix = MetricsIntakeUrlPrefix::new(
Some(site),
MetricsIntakeUrlPrefixOverride::maybe_new(dd_url, dd_dd_url),
)
.map_err(|e| {
Error::new(
std::io::ErrorKind::InvalidData,
format!("Failed to create intake url prefix: {e:?}"),
)
})?;

enable_logging_subsystem(&config);
let client = reqwest::Client::builder().no_proxy().build().map_err(|e| {
Error::new(
Expand All @@ -176,7 +212,16 @@ async fn main() -> Result<()> {
.map_err(|e| Error::new(std::io::ErrorKind::InvalidData, e.to_string()))?;

if let Some(resolved_api_key) = resolve_secrets(Arc::clone(&config), &aws_config).await {
match extension_loop_active(&aws_config, &config, &client, &r, resolved_api_key).await {
match extension_loop_active(
&aws_config,
&config,
&client,
&r,
resolved_api_key,
metrics_intake_url_prefix,
)
.await
{
Ok(()) => {
debug!("Extension loop completed successfully");
Ok(())
Expand Down Expand Up @@ -260,6 +305,7 @@ async fn extension_loop_active(
client: &Client,
r: &RegisterResponse,
resolved_api_key: String,
metrics_intake_url_prefix: MetricsIntakeUrlPrefix,
) -> Result<()> {
let mut event_bus = EventBus::run();

Expand All @@ -284,13 +330,13 @@ async fn extension_loop_active(
)
.expect("failed to create aggregator"),
));
let mut metrics_flusher = MetricsFlusher::new(
resolved_api_key.clone(),
Arc::clone(&metrics_aggr),
build_fqdn_metrics(config.site.clone()),
config.https_proxy.clone(),
Duration::from_secs(config.flush_timeout),
);
let mut metrics_flusher = MetricsFlusher::new(MetricsFlusherConfig {
api_key: resolved_api_key.clone(),
aggregator: Arc::clone(&metrics_aggr),
metrics_intake_url_prefix,
https_proxy: config.https_proxy.clone(),
timeout: Duration::from_secs(config.flush_timeout),
});

let trace_flusher = Arc::new(trace_flusher::ServerlessTraceFlusher {
buffer: Arc::new(TokioMutex::new(Vec::new())),
Expand Down
14 changes: 9 additions & 5 deletions bottlecap/src/config/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ pub struct FallbackConfig {
otlp_config_receiver_protocols_http_endpoint: Option<String>,
otlp_config_receiver_protocols_grpc_endpoint: Option<String>,
// intake urls
url: Option<String>,
dd_url: Option<String>,
Copy link
Contributor Author

@apiarian-datadog apiarian-datadog Jan 16, 2025

Choose a reason for hiding this comment

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

is this the right way to move configs from fallback into a fully supported state?

logs_config_logs_dd_url: Option<String>,
// APM, as opposed to logs, does not use the `apm_config` prefix for env vars
apm_dd_url: Option<String>,
Expand Down Expand Up @@ -107,6 +105,11 @@ pub struct Config {
pub trace_propagation_style_extract: Vec<TracePropagationStyle>,
pub trace_propagation_extract_first: bool,
pub trace_propagation_http_baggage_enabled: bool,
// Intake URLs
// These two are for metrics intake. The official config is `DD_DD_URL` but `DD_URL` can also
// be used for backwards compatibility. `DD_DD_URL` takes precedence.
pub url: Option<String>,
Copy link
Contributor

@astuyve astuyve Jan 17, 2025

Choose a reason for hiding this comment

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

The next-gen extension is already a breaking change, can we just drop this and add it to a breaking change note in the readme?

Copy link
Contributor

@duncanista duncanista Jan 17, 2025

Choose a reason for hiding this comment

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

@astuyve what about customers using old configuration? we just fallback? what's the politics behind this on how we manage a different configuration?

Copy link
Contributor

@alexgallotta alexgallotta Jan 17, 2025

Choose a reason for hiding this comment

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

If it's somehow ok to remove that deprecated config, I would say to go for it.
Worst case, the few who will complain will have a quick fix

pub dd_url: Option<String>,
}

impl Default for Config {
Expand Down Expand Up @@ -142,6 +145,9 @@ impl Default for Config {
trace_propagation_style_extract: vec![],
trace_propagation_extract_first: false,
trace_propagation_http_baggage_enabled: false,
// Intake URLs
url: None,
dd_url: None,
}
}
}
Expand Down Expand Up @@ -208,9 +214,7 @@ fn fallback(figment: &Figment, yaml_figment: &Figment) -> Result<(), ConfigError
}

// Intake URLs
if config.url.is_some()
|| config.dd_url.is_some()
|| config.logs_config_logs_dd_url.is_some()
if config.logs_config_logs_dd_url.is_some()
|| config.apm_dd_url.is_some()
|| yaml_config
.logs_config
Expand Down
24 changes: 16 additions & 8 deletions bottlecap/tests/metrics_integration_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use bottlecap::config::Config;
use bottlecap::metrics::enhanced::lambda::Lambda as enhanced_metrics;
use dogstatsd::aggregator::Aggregator as MetricsAggregator;
use dogstatsd::flusher::Flusher as MetricsFlusher;
use dogstatsd::datadog::{DdDdUrl, MetricsIntakeUrlPrefix, MetricsIntakeUrlPrefixOverride};
use dogstatsd::flusher::{Flusher as MetricsFlusher, FlusherConfig as MetricsFlusherConfig};
use dogstatsd::metric::SortedTags;
use httpmock::prelude::*;
use std::sync::{Arc, Mutex};
Expand Down Expand Up @@ -36,13 +37,20 @@ async fn test_enhanced_metrics() {
MetricsAggregator::new(SortedTags::parse("aTagKey:aTagValue").unwrap(), 1024)
.expect("failed to create aggregator"),
));
let mut metrics_flusher = MetricsFlusher::new(
dd_api_key.to_string(),
metrics_aggr.clone(),
server.base_url(),
None,
std::time::Duration::from_secs(5),
);
let mut metrics_flusher = MetricsFlusher::new(MetricsFlusherConfig {
api_key: dd_api_key.to_string(),
aggregator: metrics_aggr.clone(),
metrics_intake_url_prefix: MetricsIntakeUrlPrefix::new(
None,
MetricsIntakeUrlPrefixOverride::maybe_new(
None,
Some(DdDdUrl::new(server.base_url()).expect("failed to create dd url")),
),
)
.expect("failed to create metrics intake url prefix"),
https_proxy: None,
timeout: std::time::Duration::from_secs(5),
});
let lambda_enhanced_metrics =
enhanced_metrics::new(Arc::clone(&metrics_aggr), Arc::clone(&arc_config));

Expand Down
Loading