feat: [SVLS-5992] support DD_URL and DD_DD_URL for metrics intake#517
feat: [SVLS-5992] support DD_URL and DD_DD_URL for metrics intake#517apiarian-datadog wants to merge 2 commits intomainfrom
Conversation
| config.https_proxy.clone(), | ||
| Duration::from_secs(config.flush_timeout), | ||
| ); | ||
| let metrics_intake_url_prefix = MetricsIntakeUrlPrefix::new( |
There was a problem hiding this comment.
this feels like it should really be part of the config but that doesn't play well with our config parsing process. still, maybe we should be computing this stuff and handling sites and overrides and such in the calling function?
There was a problem hiding this comment.
is this an opportunity to split the config_for_parsing from the config_for_using structs? c.c. @duncanista @alexgallotta
There was a problem hiding this comment.
Yeah, we maybe don't want to do this here, but in config
There was a problem hiding this comment.
yep, let's keep it all close in the config/mod.rs
There was a problem hiding this comment.
At this stage, the code should have a validated and working immutable config struct, ready to be used around/
The only exception would be for config that changes dynamically, but that must be address with a prod/consumer or other async stuff
There was a problem hiding this comment.
well, i've pushed it up into main(). will followup on a thread there.
| otlp_config_receiver_protocols_grpc_endpoint: Option<String>, | ||
| // intake urls | ||
| url: Option<String>, | ||
| dd_url: Option<String>, |
There was a problem hiding this comment.
is this the right way to move configs from fallback into a fully supported state?
| // 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>, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
@astuyve what about customers using old configuration? we just fallback? what's the politics behind this on how we manage a different configuration?
There was a problem hiding this comment.
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
apiarian-datadog
force-pushed
the
aleksandr.pasechnik/svls-5992-bottlecap-metrics-intake-url
branch
from
b85f047 to
ccba5c7
Compare
| async fn main() -> Result<()> { | ||
| let (aws_config, config) = load_configs(); | ||
|
|
||
| let site = Site::new(config.site.clone()).map_err(|e| { |
There was a problem hiding this comment.
@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?
|
Closing in favor of #585 |
4 participants
No description provided.