Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions opentelemetry-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## vNext

- `EnvResourceDetector` now percent-decodes attribute values read from
`OTEL_RESOURCE_ATTRIBUTES`, matching the W3C Baggage encoding required by the
Resource SDK specification. For example,
`OTEL_RESOURCE_ATTRIBUTES=key=hello%20world` now yields the value
`hello world`. Keys are baggage tokens and are left unchanged.
([#857](https://github.com/open-telemetry/opentelemetry-rust/issues/857))
- Bound instruments are now available for `Gauge` and `UpDownCounter` via the
new `BoundGauge<T>` and `BoundUpDownCounter<T>` types exposed by the
`opentelemetry` crate. Requires the `experimental_metrics_bound_instruments`
Expand Down
4 changes: 2 additions & 2 deletions opentelemetry-sdk/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ opentelemetry-http = { workspace = true, optional = true }
futures-channel = { workspace = true }
futures-executor = { workspace = true }
futures-util = { workspace = true, features = ["std", "sink", "async-await-macro"] }
percent-encoding = { workspace = true, optional = true }
percent-encoding = { workspace = true }
rand = { workspace = true, features = ["std", "std_rng", "small_rng", "os_rng", "thread_rng"], optional = true }
serde = { workspace = true, features = ["derive", "rc"], optional = true }
serde_json = { workspace = true, optional = true }
Expand Down Expand Up @@ -44,7 +44,7 @@ pprof = { workspace = true }

[features]
default = ["trace", "metrics", "logs", "internal-logs"]
trace = ["opentelemetry/trace", "rand", "percent-encoding"]
trace = ["opentelemetry/trace", "rand"]
jaeger_remote_sampler = ["trace", "opentelemetry-http", "http", "serde", "serde_json", "url", "experimental_async_runtime"]
logs = ["opentelemetry/logs"]
metrics = ["opentelemetry/metrics"]
Expand Down
13 changes: 10 additions & 3 deletions opentelemetry-sdk/src/resource/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
//! variables.
use crate::resource::{Resource, ResourceDetector};
use opentelemetry::{Key, KeyValue, Value};
use percent_encoding::percent_decode_str;
use std::env;

const OTEL_RESOURCE_ATTRIBUTES: &str = "OTEL_RESOURCE_ATTRIBUTES";
Expand Down Expand Up @@ -50,9 +51,13 @@ fn construct_otel_resources(s: String) -> Resource {
None => return None,
};
let key = parts.0.trim();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe we should also percent-decode keys. As per specs:

The OTEL_RESOURCE_ATTRIBUTES environment variable will contain of a list of key value pairs, represented as key1=value1,key2=value2. All attribute values MUST be considered strings. The , and = characters in keys and values MUST be percent encoded. Other characters MAY be [percent-encoded](https://datatracker.ietf.org/doc/html/rfc3986#section-2.1), e.g. values outside the ANSI characters set.

let value = parts.1.trim();
// Trim OWS *before* percent-decoding so that any encoded whitespace
// (e.g. `%20`) in the value is preserved. Per the Resource SDK spec
// the value MUST be percent-decoded; keys are baggage tokens and are
// not encoded.
let value = percent_decode_str(parts.1.trim()).decode_utf8_lossy();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

decode_utf8_lossy() accepts invalid encoded UTF-8 by inserting replacement characters. The Resource SDK spec says decoding failures should discard the entire env var and report an error. The existing baggage propagator already uses decode_utf8() and logs invalid UTF-8 instead of doing lossy decoding, so this new path should probably follow that shape.


Some(KeyValue::new(key.to_owned(), value.to_owned()))
Some(KeyValue::new(key.to_owned(), value.into_owned()))
}))
.build()
}
Expand Down Expand Up @@ -124,7 +129,7 @@ mod tests {
[
(
"OTEL_RESOURCE_ATTRIBUTES",
Some("key=value, k = v , a= x, a=z,base64=SGVsbG8sIFdvcmxkIQ=="),
Some("key=value, k = v , a= x, a=z,base64=SGVsbG8sIFdvcmxkIQ==,encoded=user%20id%3D42%2C%20name%3D%22foo%22"),
),
("IRRELEVANT", Some("20200810")),
],
Expand All @@ -140,6 +145,8 @@ mod tests {
KeyValue::new("a", "x"),
KeyValue::new("a", "z"),
KeyValue::new("base64", "SGVsbG8sIFdvcmxkIQ=="), // base64('Hello, World!')
// Values are percent-decoded per the Resource SDK spec.
KeyValue::new("encoded", "user id=42, name=\"foo\""),
])
.build()
);
Expand Down
Loading