Simplify implementation of telementry's events #7280
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| use std::sync::Arc; | ||
|
|
||
| use opentelemetry::Key; | ||
| use opentelemetry::KeyValue; | ||
| use parking_lot::Mutex; | ||
|
|
@@ -6,25 +8,35 @@ use serde::Deserialize; | |
| use tower::BoxError; | ||
|
|
||
| use crate::Context; | ||
| use crate::plugins::telemetry::config_new::conditions::Condition; | ||
| use crate::plugins::telemetry::config_new::connector::ConnectorRequest; | ||
| use crate::plugins::telemetry::config_new::connector::ConnectorResponse; | ||
| use crate::plugins::telemetry::config_new::connector::attributes::ConnectorAttributes; | ||
| use crate::plugins::telemetry::config_new::connector::selectors::ConnectorSelector; | ||
| use crate::plugins::telemetry::config_new::events::CustomEvent; | ||
| use crate::plugins::telemetry::config_new::events::CustomEvents; | ||
| use crate::plugins::telemetry::config_new::events::Event; | ||
| use crate::plugins::telemetry::config_new::events::EventLevel; | ||
| use crate::plugins::telemetry::config_new::events::StandardEvent; | ||
| use crate::plugins::telemetry::config_new::events::StandardEventConfig; | ||
| use crate::plugins::telemetry::config_new::events::log_event; | ||
| use crate::plugins::telemetry::config_new::extendable::Extendable; | ||
|
|
||
| #[derive(Clone)] | ||
| pub(crate) struct ConnectorEventRequest { | ||
| // XXX(@IvanGoncharov): As part of removing Mutex from StandardEvent I moved it here | ||
| // I think it's not nessary here but can't verify it right now, so in future can just wrap StandardEvent | ||
| pub(crate) level: EventLevel, | ||
| pub(crate) condition: Arc<Mutex<Condition<ConnectorSelector>>>, | ||
IvanGoncharov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| #[derive(Clone)] | ||
| pub(crate) struct ConnectorEventResponse { | ||
| // XXX(@IvanGoncharov): As part of removing Arc from StandardEvent I moved it here | ||
| // I think it's not nessary here but can't verify it right now, so in future can just wrap StandardEvent | ||
| pub(crate) level: EventLevel, | ||
| pub(crate) condition: Arc<Condition<ConnectorSelector>>, | ||
| } | ||
|
|
||
| #[derive(Clone, Deserialize, JsonSchema, Debug, Default)] | ||
| #[serde(deny_unknown_fields, default)] | ||
|
|
@@ -46,92 +58,67 @@ pub(crate) fn new_connector_events( | |
| let custom_events = config | ||
| .custom | ||
| .iter() | ||
| .filter_map(|(name, config)| CustomEvent::from_config(name, config)) | ||
|
Contributor
Author
There was a problem hiding this comment. These functions are very repetitive, so I just moved the code to
Contributor
There was a problem hiding this comment. Maybe we could still use
Contributor
Author
There was a problem hiding this comment. TryFrom returns Result: https://doc.rust-lang.org/std/convert/trait.TryFrom.html
Contributor
Author
There was a problem hiding this comment. basically we return None if event is |
||
| .collect(); | ||
|
|
||
| ConnectorEvents { | ||
| request: StandardEvent::from_config(&config.attributes.request), | ||
| response: StandardEvent::from_config(&config.attributes.response), | ||
| error: StandardEvent::from_config(&config.attributes.error), | ||
| custom: custom_events, | ||
| } | ||
| } | ||
|
|
||
| impl CustomEvents<ConnectorRequest, ConnectorResponse, (), ConnectorAttributes, ConnectorSelector> { | ||
|
Contributor
Author
There was a problem hiding this comment. I removed the |
||
| pub(crate) fn on_request(&mut self, request: &ConnectorRequest) { | ||
| // Any condition on the request is NOT evaluated here. It must be evaluated later when | ||
| // getting the ConnectorEventRequest from the context. The request context is shared | ||
| // between all connector requests, so any request could find this ConnectorEventRequest in | ||
| // the context. Its presence on the context cannot be conditional on an individual request. | ||
| if let Some(request_event) = self.request.take() { | ||
|
Contributor
Author
There was a problem hiding this comment. I use |
||
| request.context.extensions().with_lock(|lock| { | ||
| lock.insert(ConnectorEventRequest { | ||
| level: request_event.level, | ||
| condition: Arc::new(Mutex::new(request_event.condition)), | ||
|
Contributor
Author
There was a problem hiding this comment. As noted earlier I don't think we really need this wrapping, but I don't have time to check so I move it here just to be 100% sure I'm not introducing regression here. |
||
| }) | ||
| }); | ||
| } | ||
|
|
||
| if let Some(response_event) = self.response.take() { | ||
| request.context.extensions().with_lock(|lock| { | ||
| lock.insert(ConnectorEventResponse { | ||
| level: response_event.level, | ||
| condition: Arc::new(response_event.condition), | ||
| }) | ||
| }); | ||
| } | ||
|
|
||
| for custom_event in &mut self.custom { | ||
|
Contributor
Author
There was a problem hiding this comment. Everything is mutable now because I removed the internal wrapping of Arc + Mutex around the condition. |
||
| custom_event.on_request(request); | ||
| } | ||
| } | ||
|
|
||
| pub(crate) fn on_response(&mut self, response: &ConnectorResponse) { | ||
|
Contributor
Author
There was a problem hiding this comment. Need to make it public since I removed trait |
||
| for custom_event in &mut self.custom { | ||
| custom_event.on_response(response); | ||
| } | ||
| } | ||
|
|
||
| pub(crate) fn on_error(&mut self, error: &BoxError, ctx: &Context) { | ||
| if let Some(error_event) = &mut self.error { | ||
|
Contributor
There was a problem hiding this comment. Why don't we call
Contributor
Author
There was a problem hiding this comment. Technically, we can, if we sure it's the last call (which it probably is). |
||
| if error_event.condition.evaluate_error(error, ctx) { | ||
| log_event( | ||
| error_event.level, | ||
| "connector.http.error", | ||
| vec![KeyValue::new( | ||
| Key::from_static_str("error"), | ||
| opentelemetry::Value::String(error.to_string().into()), | ||
| )], | ||
| "", | ||
| ); | ||
| } | ||
| } | ||
| for custom_event in &mut self.custom { | ||
| custom_event.on_error(error, ctx); | ||
| } | ||
| } | ||
|
|
||
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.
Whole change is caused by my crating separate enum type just for config