Skip to content

feat(upload): Add global config default logging #6019

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

Merged
Dav1dde merged 6 commits into from
May 29, 2026
+27 −9
Merged

feat(upload): Add global config default logging #6019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
984c113
feat(upload): add global config default logging
klochek May 25, 2026
60a4b3b
only log killswitched defaults
klochek May 26, 2026
80e018e
typo
klochek May 26, 2026
4f82fcd
change global_config to return an option; log when we see none
klochek May 26, 2026
d07ae7c
missing default
klochek May 26, 2026
55b6615
pr feedback
klochek May 28, 2026
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
7 changes: 7 additions & 0 deletions relay-dynamic-config/src/global.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,12 @@ fn is_err_or_empty(filters_config: &ErrorBoundary<GenericFiltersConfig>) -> bool
}
}

// Temporary until we understand why we see false killswitch values sometimes appearing.
fn default_killswitched() -> bool {
relay_log::info!("using default for endpoint fetch config");
bool::default()
Comment on lines +100 to +103
Copy link
Copy Markdown

@sentry sentry Bot May 28, 2026

Choose a reason for hiding this comment

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

Bug: The default_killswitched function logs a message on every config deserialization (every 10s) when a field is absent, causing a log flood.
Severity: MEDIUM

Suggested Fix

Remove the info! log statement from the default_killswitched function. Functions used by serde to provide default values should be free of side effects like logging to prevent unintended behavior during frequent deserialization.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: relay-dynamic-config/src/global.rs#L100-L103

Potential issue: The `default_killswitched` function emits an `info!` log message every
time it is called. This function is used by `serde` as the default provider for the
`endpoint_fetch_config_enabled` field during deserialization. Because the global config
is fetched every 10 seconds and the field is configured with `skip_serializing_if =
"is_default"`, the field is omitted when its value is `false`. This causes `serde` to
call `default_killswitched` on every fetch cycle, leading to a log flood of
approximately 8,640 messages per relay instance per day in environments where this kill
switch is not explicitly enabled.

}
Copy link
Copy Markdown

@cursor cursor Bot May 26, 2026

Choose a reason for hiding this comment

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

Debug log fires on normal deserialization, not just errors

Low Severity

The default_killswitched function logs every time relay.endpoint-fetch-config.enabled is absent during deserialization — which is the normal case when the upstream hasn't configured this option. It fires every fetch cycle (~10 seconds) regardless of whether anything is actually wrong, making it unable to distinguish the "false killswitch values" bug from routine behavior. As noted in the PR discussion, this won't achieve the intended debugging goal.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 80e018e. Configure here.


/// All options passed down from Sentry to Relay.
#[derive(Default, Clone, Debug, Serialize, Deserialize, PartialEq)]
#[serde(default)]
Expand Down Expand Up @@ -172,6 +178,7 @@ pub struct Options {

/// Kill-switch for fetching project configs in endpoints.
#[serde(
default = "default_killswitched",
rename = "relay.endpoint-fetch-config.enabled",
deserialize_with = "default_on_error",
skip_serializing_if = "is_default"
Expand Down
1 change: 1 addition & 0 deletions relay-server/src/endpoints/minidump.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,7 @@ async fn upload_context<'a>(
if !state
.global_config_handle()
.current()
.unwrap_or_default()
.options
.endpoint_fetch_config_enabled
{
Expand Down
1 change: 1 addition & 0 deletions relay-server/src/endpoints/playstation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ async fn upload_context<'a>(
if !state
.global_config_handle()
.current()
.unwrap_or_default()
.options
.endpoint_fetch_config_enabled
{
Expand Down
2 changes: 1 addition & 1 deletion relay-server/src/endpoints/unreal.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl UnrealParams {
envelope.set_header(UNREAL_USER_HEADER, user_id);
}

let global_config = state.global_config_handle().current();
let global_config = state.global_config_handle().current().unwrap_or_default();

if global_config.options.endpoint_fetch_config_enabled {
// Ensure that we really make it here.
Expand Down
6 changes: 6 additions & 0 deletions relay-server/src/endpoints/upload.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,9 +267,15 @@ fn check_kill_switch(state: &ServiceState) -> Result<(), StatusCode> {
if !state.global_config_handle().is_ready() {
relay_log::warn!("global config not available");
}

if state.global_config_handle().current().is_none() {
relay_log::info!("check_kill_switch global config is none");
}

if !state
.global_config_handle()
.current()
.unwrap_or_default()
.options
.endpoint_fetch_config_enabled
{
Expand Down
8 changes: 4 additions & 4 deletions relay-server/src/services/global_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,11 @@ impl GlobalConfigHandle {
/// Returns the currently loaded or a default global config.
///
/// When no global config has been received from upstream yet,
/// this will return a default global config.
pub fn current(&self) -> Arc<GlobalConfig> {
/// this will return None.
pub fn current(&self) -> Option<Arc<GlobalConfig>> {
Copy link
Copy Markdown
Member

@Dav1dde Dav1dde May 27, 2026

Choose a reason for hiding this comment

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

I was thinking of adding an additional method. The intention of the global config was always, even if it's missing the defaults should be good enough for Relay to work. For example proxy mode Relays will not have a global config.

But looking at the few places we actually use current() I this might be a better API overall, make it explicit to the caller.

match &*self.watch.borrow() {
Status::Ready(config) => Arc::clone(config),
Status::Pending => Default::default(),
Status::Ready(config) => Some(Arc::clone(config)),
Status::Pending => None,
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions relay-server/src/services/processor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1346,7 +1346,7 @@ impl EnvelopeProcessorService {
ManagedEnvelope::new(envelope, self.inner.addrs.outcome_aggregator.clone());
envelope.scope(scoping);

let global_config = self.inner.global_config.current();
let global_config = self.inner.global_config.current().unwrap_or_default();

let ctx = processing::Context {
config: &self.inner.config,
Expand Down Expand Up @@ -1687,7 +1687,7 @@ impl EnvelopeProcessorService {
return buckets;
};

let global_config = self.inner.global_config.current();
let global_config = self.inner.global_config.current().unwrap_or_default();
let namespaces = buckets
.iter()
.filter_map(|bucket| bucket.name.try_namespace())
Expand Down Expand Up @@ -1747,7 +1747,7 @@ impl EnvelopeProcessorService {
let scoping = *bucket_limiter.scoping();

if let Some(rate_limiter) = self.inner.rate_limiter.as_ref() {
let global_config = self.inner.global_config.current();
let global_config = self.inner.global_config.current().unwrap_or_default();
let quotas = CombinedQuotas::new(&global_config, bucket_limiter.quotas());

// We set over_accept_once such that the limit is actually reached, which allows subsequent
Expand Down
5 changes: 4 additions & 1 deletion relay-server/src/services/store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ impl StoreService {
let batch_size = self.config.metrics_max_batch_size_bytes();
let mut error = None;

let global_config = self.global_config.current();
let global_config = self.global_config.current().unwrap_or_default();
let mut encoder = BucketEncoder::new(&global_config);

let emit_sessions_to_eap = utils::is_rolled_out(
Expand Down Expand Up @@ -727,6 +727,7 @@ impl StoreService {
scoping.organization_id.value(),
self.global_config
.current()
.unwrap_or_default()
.options
.eap_outcomes_rollout_rate,
)
Expand Down Expand Up @@ -774,6 +775,7 @@ impl StoreService {
scoping.organization_id.value(),
self.global_config
.current()
.unwrap_or_default()
.options
.eap_span_outcomes_rollout_rate,
)
Expand Down Expand Up @@ -1303,6 +1305,7 @@ impl StoreService {
scoping.organization_id.value(),
self.global_config
.current()
.unwrap_or_default()
.options
.eap_span_outcomes_rollout_rate,
)
Expand Down
Loading