Skip to content

Commit

Permalink
Global error handler cleanup - Jaeger Remote sampler (#2257)
Browse files Browse the repository at this point in the history
  • Loading branch information
lalitb authored Oct 29, 2024
1 parent 40b1869 commit a5e2061
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 13 deletions.
12 changes: 7 additions & 5 deletions opentelemetry-sdk/src/trace/sampler/jaeger_remote/rate_limit.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use opentelemetry::trace::TraceError;
use std::time::SystemTime;

// leaky bucket based rate limit
Expand All @@ -9,6 +8,7 @@ pub(crate) struct LeakyBucket {
bucket_size: f64,
last_time: SystemTime,
}
use opentelemetry::otel_debug;

impl LeakyBucket {
pub(crate) fn new(bucket_size: f64, span_per_sec: f64) -> LeakyBucket {
Expand Down Expand Up @@ -53,10 +53,12 @@ impl LeakyBucket {
false
}
}
Err(_) => {
opentelemetry::global::handle_error(TraceError::Other(
"jaeger remote sampler gets rewinded timestamp".into(),
));
Err(err) => {
otel_debug!(
name: "JaegerRemoteSampler.LeakyBucket.ClockAdjustment",
message = "Jaeger remote sampler detected a rewind in system clock",
reason = format!("{:?}", err),
);
true
}
}
Expand Down
10 changes: 8 additions & 2 deletions opentelemetry-sdk/src/trace/sampler/jaeger_remote/sampler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::trace::{Sampler, ShouldSample};
use futures_util::{stream, StreamExt as _};
use http::Uri;
use opentelemetry::trace::{Link, SamplingResult, SpanKind, TraceError, TraceId};
use opentelemetry::{global, Context, KeyValue};
use opentelemetry::{otel_warn, Context, KeyValue};
use opentelemetry_http::HttpClient;
use std::str::FromStr;
use std::sync::Arc;
Expand Down Expand Up @@ -203,7 +203,13 @@ impl JaegerRemoteSampler {
// send request
match Self::request_new_strategy(&client, endpoint.clone()).await {
Ok(remote_strategy_resp) => strategy.update(remote_strategy_resp),
Err(err_msg) => global::handle_error(TraceError::Other(err_msg.into())),
Err(err_msg) => {
otel_warn!(
name: "JaegerRemoteSampler.FailedToFetchStrategy",
message= "Failed to fetch the sampling strategy from the remote endpoint. The last successfully fetched configuration will be used if available; otherwise, the default sampler will be applied until a successful configuration fetch.",
reason = format!("{}", err_msg),
);
}
};
} else {
// shutdown
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use crate::trace::sampler::jaeger_remote::remote::{
};
use crate::trace::sampler::sample_based_on_probability;
use opentelemetry::trace::{
SamplingDecision, SamplingResult, TraceContextExt, TraceError, TraceId, TraceState,
SamplingDecision, SamplingResult, TraceContextExt, TraceId, TraceState,
};
use opentelemetry::{global, Context};
use opentelemetry::{otel_warn, Context};
use std::collections::HashMap;
use std::fmt::{Debug, Formatter};
use std::sync::Mutex;
Expand Down Expand Up @@ -107,9 +107,10 @@ impl Inner {
}
})
.unwrap_or_else(|_err| {
global::handle_error(TraceError::Other(
"jaeger remote sampler mutex poisoned".into(),
))
otel_warn!(
name: "JaegerRemoteSampler.MutexPoisoned",
message = "Unable to update Jaeger Remote sampling strategy: the sampler's internal mutex is poisoned, likely due to a panic in another thread holding the lock. No further attempts to update the strategy will be made until the application or process restarts, and the last known configuration will continue to be used.",
);
});
}

Expand Down Expand Up @@ -137,7 +138,13 @@ impl Inner {
(_, _, Some(probabilistic)) => {
Some(Strategy::Probabilistic(probabilistic.sampling_rate))
}
_ => None,
_ => {
otel_warn!(
name: "JaegerRemoteSampler.InvalidStrategyReceived",
message = "Invalid sampling strategy received from the remote endpoint. Expected one of: OperationSampling, RateLimitingSampling, or ProbabilisticSampling. Continuing to use the previous strategy or default sampler until a successful update.",
);
None
}
}
}

Expand Down

0 comments on commit a5e2061

Please sign in to comment.