-
Notifications
You must be signed in to change notification settings - Fork 40
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
Prepare crates for otel v0.27.0 #130
Conversation
@@ -17,6 +17,7 @@ impl IntoJson for AnyValue { | |||
AnyValue::Bytes(_value) => todo!("No support for AnyValue::Bytes yet."), | |||
AnyValue::ListAny(value) => value.as_json_value(), | |||
AnyValue::Map(value) => value.as_json_value(), | |||
&_ => Value::Null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I rather we add a branch that explicitly calls todo!
. This way, upgrades to opentelemetry that adds a new value will fail to compile, forcing us to add support for it. Returning Null
make it easy to forget.
&_ => Value::Null, | |
&_ => { | |
// If we reach this line is because we are upgrading opentelemetry to a version that contains a new type. | |
// We must add support for the new type. | |
todo!("Add support for new types"); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@psandana The goal is to ensure that adding a new enum variant in OpenTelemetry does not break custom exporters.
PS: I believe the suggested change won't cause a compile-time failure but will result in a runtime panic, which we want to avoid entirely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lalitb you are right. IMO, if compile error is preferred, then I would just remove the &_
entry. If opentelemetry ever adds a new item to the enum, it should come with code that handles it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad. Forgot this is non-exhaustive, so you must use a default branch.
@@ -401,6 +401,7 @@ fn add_attribute_to_event(event: &mut tld::EventBuilder, key: &Key, value: &AnyV | |||
0, | |||
); | |||
} | |||
&_ => {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as in converters.rs file:
&_ => {} | |
&_ => { | |
// If we reach this line is because we are upgrading opentelemetry to a version that contains a new type. | |
// We must add support for the new type. | |
todo!("Add support for new types"); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@psandana same as the earlier comment.
crates are published now: ~/obs/ot/opentelemetry-rust-contrib$ for crate in opentelemetry-*; do
data=$(curl -s "https://crates.io/api/v1/crates/$crate")
latest_version=$(echo "$data" | jq -r '.versions[0].num')
latest_date=$(echo "$data" | jq -r '.versions[0].created_at' | xargs -I{} date -d {} +"%Y-%m-%d %H:%M:%S")
echo "$crate: version $latest_version, published at $latest_date"
done
opentelemetry-aws: version 0.15.0, published at 2024-11-25 22:44:15
opentelemetry-contrib: version 0.19.0, published at 2024-11-25 22:45:29
opentelemetry-datadog: version 0.15.0, published at 2024-11-25 22:46:17
opentelemetry-etw-logs: version 0.6.0, published at 2024-11-25 22:46:37
opentelemetry-etw-metrics: version 0.6.0, published at 2024-11-25 22:47:17
opentelemetry-resource-detectors: version 0.6.0, published at 2024-11-25 22:47:52
opentelemetry-stackdriver: version 0.24.0, published at 2024-11-25 22:48:33
opentelemetry-user-events-logs: version 0.8.0, published at 2024-11-25 22:49:18
opentelemetry-user-events-metrics: version 0.8.0, published at 2024-11-25 22:49:02
opentelemetry-zpages: version 0.12.0, published at 2024-11-25 22:50:04 |
Fixes #
Design discussion issue (if applicable) #
Changes
Please provide a brief description of the changes here.
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes