Skip to content
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.

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

Global error handler cleanup - Jaeger Remote sampler #2257

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
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 Down Expand Up @@ -53,10 +52,12 @@ impl LeakyBucket {
false
}
}
Err(_) => {
opentelemetry::global::handle_error(TraceError::Other(
"jaeger remote sampler gets rewinded timestamp".into(),
));
Err(err) => {
opentelemetry::otel_debug!(
name: "JaegerRemoteSampler.LeakyBucket.ClockAdjustment",
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
message = "jaeger remote sampler detected a rewind in system clock",
reason = format!("{:?}", err),
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
);
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_error, 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_error!(
name: "JaegerRemoteSampler.UpdateStrategy.RequestFailed",
lalitb marked this conversation as resolved.
Show resolved Hide resolved
message = "Failed to fetch new sampling strategy from remote endpoint. This may cause the sampler to use stale configuration until the next successful update.",
lalitb marked this conversation as resolved.
Show resolved Hide resolved
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_debug!(
lalitb marked this conversation as resolved.
Show resolved Hide resolved
name: "JaegerRemoteSampler.MutexPoisoned",
message = "Failed to update Jaeger Remote sampling strategy. The sampler's internal mutex is poisoned, indicating a panic occurred in another thread holding the lock. Sampling decisions may be using stale configuration.",
lalitb marked this conversation as resolved.
Show resolved Hide resolved
);
});
}

Expand Down Expand Up @@ -137,7 +138,15 @@ impl Inner {
(_, _, Some(probabilistic)) => {
Some(Strategy::Probabilistic(probabilistic.sampling_rate))
}
_ => None,
_ => {
otel_warn!(
name: "Sampler.JaegerRemote.InvalidStrategy",
lalitb marked this conversation as resolved.
Show resolved Hide resolved
message = "Received invalid sampling strategy from Jaeger remote endpoint. Expected one of: OperationSampling, RateLimitingSampling (max traces per second), or ProbabilisticSampling (0.0-1.0 sampling probability). No valid strategy was found in the response. Using previous strategy if available.",
lalitb marked this conversation as resolved.
Show resolved Hide resolved
received_operation_sampling = operation_sampling.is_some(),
received_rate_limiting = rate_limiting_sampling.is_some(),
received_probabilistic = probabilistic_sampling.is_some()
);
}
}
}

Expand Down
Loading