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

Allow ref counted keys and values #821

Merged
merged 2 commits into from Jun 25, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/external-otlp-grpcio-async-std/Cargo.toml
Expand Up @@ -5,7 +5,7 @@ edition = "2018"
publish = false

[dependencies]
async-std = { version = "= 1.8.0", features = ["attributes"] }
async-std = { version = "= 1.10.0", features = ["attributes"] }
env_logger = "0.8.2"
opentelemetry = { path = "../../opentelemetry", features = ["rt-async-std"] }
opentelemetry-otlp = { path = "../../opentelemetry-otlp", features = [
Expand Down
2 changes: 1 addition & 1 deletion examples/external-otlp-tonic-tokio/src/main.rs
Expand Up @@ -62,7 +62,7 @@ fn init_tracer() -> Result<sdktrace::Tracer, TraceError> {
opentelemetry_otlp::new_exporter()
.tonic()
.with_endpoint(endpoint.as_str())
.with_metadata(dbg!(metadata))
.with_metadata(metadata)
.with_tls_config(
ClientTlsConfig::new().domain_name(
endpoint
Expand Down
234 changes: 209 additions & 25 deletions opentelemetry-api/src/common.rs
@@ -1,23 +1,35 @@
use std::borrow::Cow;
use std::fmt;
use std::sync::Arc;
use std::{fmt, hash};

/// The key part of attribute [KeyValue] pairs.
///
/// See the [attribute naming] spec for guidelines.
///
/// [attribute naming]: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.9.0/specification/common/attribute-naming.md
#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct Key(Cow<'static, str>);
#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub struct Key(OtelString);

impl Key {
/// Create a new `Key`.
pub fn new<S: Into<Cow<'static, str>>>(value: S) -> Self {
Key(value.into())
///
/// # Examples
///
/// ```
/// use opentelemetry_api::Key;
/// use std::sync::Arc;
///
/// let key1 = Key::new("my_static_str");
/// let key2 = Key::new(String::from("my_owned_string"));
/// let key3 = Key::new(Arc::from("my_ref_counted_str"));
/// ```
pub fn new(value: impl Into<Key>) -> Self {
value.into()
}

/// Create a new const `Key`.
pub const fn from_static_str(value: &'static str) -> Self {
Key(Cow::Borrowed(value))
Key(OtelString::Static(value))
}

/// Create a `KeyValue` pair for `bool` values.
Expand All @@ -44,8 +56,8 @@ impl Key {
}
}

/// Create a `KeyValue` pair for `String` values.
pub fn string<T: Into<Cow<'static, str>>>(self, value: T) -> KeyValue {
/// Create a `KeyValue` pair for string-like values.
pub fn string(self, value: impl Into<StringValue>) -> KeyValue {
KeyValue {
key: self,
value: Value::String(value.into()),
Expand All @@ -62,34 +74,95 @@ impl Key {

/// Returns a reference to the underlying key name
pub fn as_str(&self) -> &str {
self.0.as_ref()
self.0.as_str()
}
}

impl From<&'static str> for Key {
/// Convert a `&str` to a `Key`.
fn from(key_str: &'static str) -> Self {
Key(Cow::from(key_str))
Key(OtelString::Static(key_str))
}
}

impl From<String> for Key {
/// Convert a `String` to a `Key`.
fn from(string: String) -> Self {
Key(Cow::from(string))
Key(OtelString::Owned(string))
}
}

impl From<Arc<str>> for Key {
/// Convert a `String` to a `Key`.
fn from(string: Arc<str>) -> Self {
Key(OtelString::RefCounted(string))
}
}

impl fmt::Debug for Key {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
self.0.fmt(fmt)
}
}

impl From<Key> for String {
/// Converts `Key` instances into `String`.
fn from(key: Key) -> Self {
key.0.into_owned()
match key.0 {
OtelString::Owned(s) => s,
OtelString::Static(s) => s.to_string(),
OtelString::RefCounted(s) => s.to_string(),
}
}
}

impl fmt::Display for Key {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
self.0.fmt(fmt)
match &self.0 {
OtelString::Owned(s) => s.fmt(fmt),
OtelString::Static(s) => s.fmt(fmt),
OtelString::RefCounted(s) => s.fmt(fmt),
}
}
}

#[derive(Clone, Debug, Eq)]
enum OtelString {
Static(&'static str),
Owned(String),
RefCounted(Arc<str>),
}

impl OtelString {
fn as_str(&self) -> &str {
match self {
OtelString::Owned(s) => s.as_ref(),
OtelString::Static(s) => s,
OtelString::RefCounted(s) => s.as_ref(),
}
}
}

impl PartialOrd for OtelString {
fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
self.as_str().partial_cmp(other.as_str())
}
}

impl Ord for OtelString {
fn cmp(&self, other: &Self) -> std::cmp::Ordering {
self.as_str().cmp(other.as_str())
}
}

impl PartialEq for OtelString {
fn eq(&self, other: &Self) -> bool {
self.as_str().eq(other.as_str())
}
}

impl hash::Hash for OtelString {
fn hash<H: hash::Hasher>(&self, state: &mut H) {
self.as_str().hash(state)
}
}

Expand All @@ -103,7 +176,7 @@ pub enum Array {
/// Array of floats
F64(Vec<f64>),
/// Array of strings
String(Vec<Cow<'static, str>>),
String(Vec<StringValue>),
}

impl fmt::Display for Array {
Expand All @@ -118,7 +191,7 @@ impl fmt::Display for Array {
if i > 0 {
write!(fmt, ",")?;
}
write!(fmt, "{:?}", t)?;
write!(fmt, "\"{}\"", t)?;
}
write!(fmt, "]")
}
Expand Down Expand Up @@ -153,9 +226,42 @@ into_array!(
(Vec<bool>, Array::Bool),
(Vec<i64>, Array::I64),
(Vec<f64>, Array::F64),
(Vec<Cow<'static, str>>, Array::String),
(Vec<StringValue>, Array::String),
);

impl From<Vec<&'static str>> for Array {
/// Convenience method for creating a `Value` from a `&'static str`.
Copy link
Contributor

Choose a reason for hiding this comment

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

The comments here are slightly wrong/misleading IMO.

Are these conversions especially common across the other crates? It feels like these might make it easy to do something relatively inefficient, so maybe we should leave the conversion to particular call sites?

Copy link
Member Author

Choose a reason for hiding this comment

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

Happy to remove them

fn from(ss: Vec<&'static str>) -> Self {
Array::String(
ss.into_iter()
.map(|s| StringValue(OtelString::Static(s)))
.collect(),
)
}
}

impl From<Vec<String>> for Array {
/// Convenience method for creating a `Value` from a `String`.
fn from(ss: Vec<String>) -> Self {
Array::String(
ss.into_iter()
.map(|s| StringValue(OtelString::Owned(s)))
.collect(),
)
}
}

impl From<Vec<Arc<str>>> for Array {
/// Convenience method for creating a `Value` from a `String`.
fn from(ss: Vec<Arc<str>>) -> Self {
Array::String(
ss.into_iter()
.map(|s| StringValue(OtelString::RefCounted(s)))
.collect(),
)
}
}

/// The value part of attribute [KeyValue] pairs.
#[derive(Clone, Debug, PartialEq)]
pub enum Value {
Expand All @@ -166,11 +272,75 @@ pub enum Value {
/// f64 values
F64(f64),
/// String values
String(Cow<'static, str>),
String(StringValue),
/// Array of homogeneous values
Array(Array),
}

/// Wrapper for string-like values
#[derive(Clone, PartialEq, Hash)]
pub struct StringValue(OtelString);

impl fmt::Debug for StringValue {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.0.fmt(f)
}
}

impl fmt::Display for StringValue {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match &self.0 {
OtelString::Owned(s) => s.fmt(f),
OtelString::Static(s) => s.fmt(f),
OtelString::RefCounted(s) => s.fmt(f),
}
}
}

impl StringValue {
/// Returns a string slice to this value
pub fn as_str(&self) -> &str {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably also implement AsRef<str>.

self.0.as_str()
}
}

impl From<StringValue> for String {
fn from(s: StringValue) -> Self {
match s.0 {
OtelString::Owned(s) => s,
OtelString::Static(s) => s.to_string(),
OtelString::RefCounted(s) => s.to_string(),
}
}
}

impl From<&'static str> for StringValue {
fn from(s: &'static str) -> Self {
StringValue(OtelString::Static(s))
}
}

impl From<String> for StringValue {
fn from(s: String) -> Self {
StringValue(OtelString::Owned(s))
}
}

impl From<Arc<str>> for StringValue {
fn from(s: Arc<str>) -> Self {
StringValue(OtelString::RefCounted(s))
}
}

impl From<Cow<'static, str>> for StringValue {
fn from(s: Cow<'static, str>) -> Self {
match s {
Cow::Owned(s) => StringValue(OtelString::Owned(s)),
Cow::Borrowed(s) => StringValue(OtelString::Static(s)),
}
}
}

impl Value {
/// String representation of the `Value`
///
Expand All @@ -180,7 +350,7 @@ impl Value {
Value::Bool(v) => format!("{}", v).into(),
Value::I64(v) => format!("{}", v).into(),
Value::F64(v) => format!("{}", v).into(),
Value::String(v) => Cow::Borrowed(v.as_ref()),
Value::String(v) => Cow::Borrowed(v.as_str()),
Value::Array(v) => format!("{}", v).into(),
}
}
Expand All @@ -206,7 +376,7 @@ from_values!(
(bool, Value::Bool);
(i64, Value::I64);
(f64, Value::F64);
(Cow<'static, str>, Value::String);
(StringValue, Value::String);
);

impl From<&'static str> for Value {
Expand All @@ -223,14 +393,28 @@ impl From<String> for Value {
}
}

impl From<Arc<str>> for Value {
/// Convenience method for creating a `Value` from a `String`.
fn from(s: Arc<str>) -> Self {
Value::String(s.into())
}
}

impl From<Cow<'static, str>> for Value {
/// Convenience method for creating a `Value` from a `String`.
fn from(s: Cow<'static, str>) -> Self {
Value::String(s.into())
}
}

impl fmt::Display for Value {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Value::Bool(v) => fmt.write_fmt(format_args!("{}", v)),
Value::I64(v) => fmt.write_fmt(format_args!("{}", v)),
Value::F64(v) => fmt.write_fmt(format_args!("{}", v)),
Value::String(v) => fmt.write_fmt(format_args!("{}", v)),
Value::Array(v) => fmt.write_fmt(format_args!("{}", v)),
Value::Bool(v) => v.fmt(fmt),
Value::I64(v) => v.fmt(fmt),
Value::F64(v) => v.fmt(fmt),
Value::String(v) => fmt.write_str(v.as_str()),
Value::Array(v) => v.fmt(fmt),
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-contrib/Cargo.toml
Expand Up @@ -34,7 +34,7 @@ serde_json = { version = "1", optional = true}
futures = {version = "0.3", optional = true}
async-trait = {version = "0.1", optional = true}
tokio = { version = "1.0", features = ["fs", "io-util"], optional = true }
async-std = { version = "1.6", optional = true }
async-std = { version = "1.10", optional = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

Although it probably won't break the MSRV, we should probably pin this to 1.10 to be consistent with other crates?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was only to get CI passing. Still have to resolve the MSRV issues on main.



[dev-dependencies]
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-contrib/src/trace/exporter/jaeger_json.rs
Expand Up @@ -193,7 +193,7 @@ fn opentelemetry_value_to_json(value: &opentelemetry::Value) -> (&str, serde_jso
opentelemetry::Value::Bool(b) => ("bool", serde_json::json!(b)),
opentelemetry::Value::I64(i) => ("int64", serde_json::json!(i)),
opentelemetry::Value::F64(f) => ("float64", serde_json::json!(f)),
opentelemetry::Value::String(s) => ("string", serde_json::json!(s)),
opentelemetry::Value::String(s) => ("string", serde_json::json!(s.as_str())),
v @ opentelemetry::Value::Array(_) => ("string", serde_json::json!(v.to_string())),
}
}
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-datadog/src/exporter/model/v03.rs
Expand Up @@ -41,7 +41,7 @@ where
if let Some(Value::String(s)) = span.attributes.get(&Key::new("span.type")) {
rmp::encode::write_map_len(&mut encoded, 11)?;
rmp::encode::write_str(&mut encoded, "type")?;
rmp::encode::write_str(&mut encoded, s.as_ref())?;
rmp::encode::write_str(&mut encoded, s.as_str())?;
} else {
rmp::encode::write_map_len(&mut encoded, 10)?;
}
Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-datadog/src/exporter/model/v05.rs
Expand Up @@ -121,7 +121,7 @@ where
.unwrap_or(0);

let span_type = match span.attributes.get(&Key::new("span.type")) {
Some(Value::String(s)) => interner.intern(s.as_ref()),
Some(Value::String(s)) => interner.intern(s.as_str()),
_ => interner.intern(""),
};

Expand Down