Skip to content

Commit e99195b

Browse files
lalitblalitb
authored
Merge branch 'main' into global-error-handler-cleanup-logs-sdk
2 parents f695605 + bacb3da commit e99195b

File tree

22 files changed

+384
-151
lines changed

22 files changed

+384
-151
lines changed

‎opentelemetry-appender-tracing/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ tracing-subscriber = { workspace = true, features = ["registry", "std", "env-fil
2626
tracing-log = "0.2"
2727
async-trait = { workspace = true }
2828
criterion = { workspace = true }
29+
tokio = { workspace = true, features = ["full"]}
2930

3031
[target.'cfg(not(target_os = "windows"))'.dev-dependencies]
3132
pprof = { version = "0.13", features = ["flamegraph", "criterion"] }

‎opentelemetry-appender-tracing/src/layer.rs

+70-3
Original file line numberDiff line numberDiff line change
@@ -208,17 +208,20 @@ const fn severity_of_level(level: &Level) -> Severity {
208208
#[cfg(test)]
209209
mod tests {
210210
use crate::layer;
211-
use opentelemetry::logs::Severity;
211+
use async_trait::async_trait;
212+
use opentelemetry::logs::{LogResult, Severity};
212213
use opentelemetry::trace::TracerProvider as _;
213214
use opentelemetry::trace::{TraceContextExt, TraceFlags, Tracer};
214215
use opentelemetry::{logs::AnyValue, Key};
216+
use opentelemetry_sdk::export::logs::{LogBatch, LogExporter};
215217
use opentelemetry_sdk::logs::{LogRecord, LoggerProvider};
216218
use opentelemetry_sdk::testing::logs::InMemoryLogsExporter;
217219
use opentelemetry_sdk::trace;
218220
use opentelemetry_sdk::trace::{Sampler, TracerProvider};
219-
use tracing::error;
221+
use tracing::{error, warn};
220222
use tracing_subscriber::prelude::__tracing_subscriber_SubscriberExt;
221-
use tracing_subscriber::Layer;
223+
use tracing_subscriber::util::SubscriberInitExt;
224+
use tracing_subscriber::{EnvFilter, Layer};
222225

223226
pub fn attributes_contains(log_record: &LogRecord, key: &Key, value: &AnyValue) -> bool {
224227
log_record
@@ -238,6 +241,70 @@ mod tests {
238241
}
239242

240243
// cargo test --features=testing
244+
245+
#[derive(Clone, Debug, Default)]
246+
struct ReentrantLogExporter;
247+
248+
#[async_trait]
249+
impl LogExporter for ReentrantLogExporter {
250+
async fn export(&mut self, _batch: LogBatch<'_>) -> LogResult<()> {
251+
// This will cause a deadlock as the export itself creates a log
252+
// while still within the lock of the SimpleLogProcessor.
253+
warn!(name: "my-event-name", target: "reentrant", event_id = 20, user_name = "otel", user_email = "otel@opentelemetry.io");
254+
Ok(())
255+
}
256+
}
257+
258+
#[test]
259+
#[ignore = "See issue: https://github.com/open-telemetry/opentelemetry-rust/issues/1745"]
260+
fn simple_processor_deadlock() {
261+
let exporter: ReentrantLogExporter = ReentrantLogExporter;
262+
let logger_provider = LoggerProvider::builder()
263+
.with_simple_exporter(exporter.clone())
264+
.build();
265+
266+
let layer = layer::OpenTelemetryTracingBridge::new(&logger_provider);
267+
268+
// Setting subscriber as global as that is the only way to test this scenario.
269+
tracing_subscriber::registry().with(layer).init();
270+
warn!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "otel@opentelemetry.io");
271+
}
272+
273+
#[test]
274+
#[ignore = "While this test runs fine, this uses global subscriber and does not play well with other tests."]
275+
fn simple_processor_no_deadlock() {
276+
let exporter: ReentrantLogExporter = ReentrantLogExporter;
277+
let logger_provider = LoggerProvider::builder()
278+
.with_simple_exporter(exporter.clone())
279+
.build();
280+
281+
let layer = layer::OpenTelemetryTracingBridge::new(&logger_provider);
282+
283+
// This filter will prevent the deadlock as the reentrant log will be
284+
// ignored.
285+
let filter = EnvFilter::new("debug").add_directive("reentrant=error".parse().unwrap());
286+
// Setting subscriber as global as that is the only way to test this scenario.
287+
tracing_subscriber::registry()
288+
.with(filter)
289+
.with(layer)
290+
.init();
291+
warn!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "otel@opentelemetry.io");
292+
}
293+
294+
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
295+
#[ignore = "While this test runs fine, this uses global subscriber and does not play well with other tests."]
296+
async fn batch_processor_no_deadlock() {
297+
let exporter: ReentrantLogExporter = ReentrantLogExporter;
298+
let logger_provider = LoggerProvider::builder()
299+
.with_batch_exporter(exporter.clone(), opentelemetry_sdk::runtime::Tokio)
300+
.build();
301+
302+
let layer = layer::OpenTelemetryTracingBridge::new(&logger_provider);
303+
304+
tracing_subscriber::registry().with(layer).init();
305+
warn!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "otel@opentelemetry.io");
306+
}
307+
241308
#[test]
242309
fn tracing_appender_standalone() {
243310
// Arrange

‎opentelemetry-proto/src/proto.rs

+16-28
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub(crate) mod serializers {
66
use crate::tonic::common::v1::any_value::{self, Value};
77
use crate::tonic::common::v1::AnyValue;
88
use serde::de::{self, MapAccess, Visitor};
9-
use serde::ser::SerializeStruct;
9+
use serde::ser::{SerializeMap, SerializeStruct};
1010
use serde::{Deserialize, Deserializer, Serialize, Serializer};
1111
use std::fmt;
1212

@@ -45,35 +45,23 @@ pub(crate) mod serializers {
4545
}
4646

4747
// AnyValue <-> KeyValue conversion
48-
pub fn serialize_to_value<S>(value: &Option<AnyValue>, serializer: S) -> Result<S::Ok, S::Error>
48+
pub fn serialize_to_value<S>(value: &Option<Value>, serializer: S) -> Result<S::Ok, S::Error>
4949
where
5050
S: Serializer,
5151
{
52-
match value {
53-
Some(any_value) => match &any_value.value {
54-
Some(Value::IntValue(i)) => {
55-
// Attempt to create a struct to wrap the intValue
56-
let mut state = match serializer.serialize_struct("Value", 1) {
57-
Ok(s) => s,
58-
Err(e) => return Err(e), // Handle the error or return it
59-
};
60-
61-
// Attempt to serialize the intValue field
62-
if let Err(e) = state.serialize_field("intValue", &i.to_string()) {
63-
return Err(e); // Handle the error or return it
64-
}
65-
66-
// Finalize the struct serialization
67-
state.end()
68-
}
69-
Some(value) => value.serialize(serializer),
70-
None => serializer.serialize_none(),
71-
},
52+
match &value {
53+
Some(Value::IntValue(i)) => {
54+
// Attempt to serialize the intValue field
55+
let mut map = serializer.serialize_map(Some(1))?;
56+
map.serialize_entry("intValue", &i.to_string());
57+
map.end()
58+
}
59+
Some(value) => value.serialize(serializer),
7260
None => serializer.serialize_none(),
7361
}
7462
}
7563

76-
pub fn deserialize_from_value<'de, D>(deserializer: D) -> Result<Option<AnyValue>, D::Error>
64+
pub fn deserialize_from_value<'de, D>(deserializer: D) -> Result<Option<Value>, D::Error>
7765
where
7866
D: Deserializer<'de>,
7967
{
@@ -99,13 +87,13 @@ pub(crate) mod serializers {
9987
}
10088

10189
impl<'de> de::Visitor<'de> for ValueVisitor {
102-
type Value = AnyValue;
90+
type Value = Option<Value>;
10391

10492
fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
10593
formatter.write_str("a JSON object for AnyValue")
10694
}
10795

108-
fn visit_map<V>(self, mut map: V) -> Result<AnyValue, V::Error>
96+
fn visit_map<V>(self, mut map: V) -> Result<Option<Value>, V::Error>
10997
where
11098
V: de::MapAccess<'de>,
11199
{
@@ -150,17 +138,17 @@ pub(crate) mod serializers {
150138
}
151139

152140
if let Some(v) = value {
153-
Ok(AnyValue { value: Some(v) })
141+
Ok(Some(v))
154142
} else {
155143
Err(de::Error::custom(
156-
"Invalid data for AnyValue, no known keys found",
144+
"Invalid data for Value, no known keys found",
157145
))
158146
}
159147
}
160148
}
161149

162150
let value = deserializer.deserialize_map(ValueVisitor)?;
163-
Ok(Some(value))
151+
Ok(value)
164152
}
165153

166154
pub fn serialize_u64_to_string<S>(value: &u64, serializer: S) -> Result<S::Ok, S::Error>

‎opentelemetry-proto/src/proto/tonic/opentelemetry.proto.common.v1.rs

+8-7
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@ pub struct AnyValue {
1010
/// The value is one of the listed fields. It is valid for all values to be unspecified
1111
/// in which case this AnyValue is considered to be "empty".
1212
#[prost(oneof = "any_value::Value", tags = "1, 2, 3, 4, 5, 6, 7")]
13+
#[cfg_attr(
14+
feature = "with-serde",
15+
serde(
16+
flatten,
17+
serialize_with = "crate::proto::serializers::serialize_to_value",
18+
deserialize_with = "crate::proto::serializers::deserialize_from_value"
19+
)
20+
)]
1321
pub value: ::core::option::Option<any_value::Value>,
1422
}
1523
/// Nested message and enum types in `AnyValue`.
@@ -75,13 +83,6 @@ pub struct KeyValue {
7583
#[prost(string, tag = "1")]
7684
pub key: ::prost::alloc::string::String,
7785
#[prost(message, optional, tag = "2")]
78-
#[cfg_attr(
79-
feature = "with-serde",
80-
serde(
81-
serialize_with = "crate::proto::serializers::serialize_to_value",
82-
deserialize_with = "crate::proto::serializers::deserialize_from_value"
83-
)
84-
)]
8586
pub value: ::core::option::Option<AnyValue>,
8687
}
8788
/// InstrumentationScope is a message representing the instrumentation scope information

‎opentelemetry-proto/src/proto/tonic/opentelemetry.proto.logs.v1.rs

-7
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,6 @@ pub struct LogRecord {
122122
/// string message (including multi-line) describing the event in a free form or it can
123123
/// be a structured data composed of arrays and maps of other values. \[Optional\].
124124
#[prost(message, optional, tag = "5")]
125-
#[cfg_attr(
126-
feature = "with-serde",
127-
serde(
128-
serialize_with = "crate::proto::serializers::serialize_to_value",
129-
deserialize_with = "crate::proto::serializers::deserialize_from_value"
130-
)
131-
)]
132125
pub body: ::core::option::Option<super::super::common::v1::AnyValue>,
133126
/// Additional attributes that describe the specific event occurrence. \[Optional\].
134127
/// Attribute keys MUST be unique (it is not allowed to have more than one

‎opentelemetry-proto/tests/grpc_build.rs

+4-5
Original file line numberDiff line numberDiff line change
@@ -111,11 +111,10 @@ fn build_tonic() {
111111
.field_attribute(path, "#[cfg_attr(feature = \"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_u64_to_string\", deserialize_with = \"crate::proto::serializers::deserialize_string_to_u64\"))]")
112112
}
113113

114-
// add custom serializer and deserializer for AnyValue
115-
for path in ["common.v1.KeyValue.value", "logs.v1.LogRecord.body"] {
116-
builder = builder
117-
.field_attribute(path, "#[cfg_attr(feature =\"with-serde\", serde(serialize_with = \"crate::proto::serializers::serialize_to_value\", deserialize_with = \"crate::proto::serializers::deserialize_from_value\"))]");
118-
}
114+
// special serializer and deserializer for value
115+
// The Value::value field must be hidden
116+
builder = builder
117+
.field_attribute("common.v1.AnyValue.value", "#[cfg_attr(feature =\"with-serde\", serde(flatten, serialize_with = \"crate::proto::serializers::serialize_to_value\", deserialize_with = \"crate::proto::serializers::deserialize_from_value\"))]");
119118

120119
// flatten
121120
for path in ["metrics.v1.Metric.data", "metrics.v1.NumberDataPoint.value"] {

‎opentelemetry-proto/tests/json_serde.rs

+8-24
Original file line numberDiff line numberDiff line change
@@ -518,14 +518,10 @@ mod json_serde {
518518
"arrayValue": {
519519
"values": [
520520
{
521-
"value": {
522-
"stringValue": "foo"
523-
}
521+
"stringValue": "foo"
524522
},
525523
{
526-
"value": {
527-
"stringValue": "bar"
528-
}
524+
"stringValue": "bar"
529525
}
530526
]
531527
}
@@ -557,14 +553,10 @@ mod json_serde {
557553
"arrayValue": {
558554
"values": [
559555
{
560-
"value": {
561-
"stringValue": "foo"
562-
}
556+
"stringValue": "foo"
563557
},
564558
{
565-
"value": {
566-
"intValue": 1337
567-
}
559+
"intValue": "1337"
568560
}
569561
]
570562
}
@@ -1339,14 +1331,10 @@ mod json_serde {
13391331
"arrayValue": {
13401332
"values": [
13411333
{
1342-
"value": {
1343-
"stringValue": "many"
1344-
}
1334+
"stringValue": "many"
13451335
},
13461336
{
1347-
"value": {
1348-
"stringValue": "values"
1349-
}
1337+
"stringValue": "values"
13501338
}
13511339
]
13521340
}
@@ -1453,14 +1441,10 @@ mod json_serde {
14531441
"arrayValue": {
14541442
"values": [
14551443
{
1456-
"value": {
1457-
"stringValue": "many"
1458-
}
1444+
"stringValue": "many"
14591445
},
14601446
{
1461-
"value": {
1462-
"stringValue": "values"
1463-
}
1447+
"stringValue": "values"
14641448
}
14651449
]
14661450
}

‎opentelemetry-sdk/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
- Bump MSRV to 1.70 [#2179](https://github.com/open-telemetry/opentelemetry-rust/pull/2179)
66
- Implement `LogRecord::set_trace_context` for `LogRecord`. Respect any trace context set on a `LogRecord` when emitting through a `Logger`.
7+
- Improved `LoggerProvider` shutdown handling to prevent redundant shutdown calls when `drop` is invoked. [#2195](https://github.com/open-telemetry/opentelemetry-rust/pull/2195)
78

89
## v0.26.0
910
Released 2024-Sep-30

0 commit comments

Comments
 (0)