Adding subscription_id and stream_response_time_ns to subscribe response metadata [ --feature stats ] #32
Conversation
| @@ -153,17 +153,49 @@ pub struct QuerySubscription { | |||
| permissions: Permissions, | |||
| } | |||
|
|
|||
| #[cfg(not(feature="stats"))] | |||
There was a problem hiding this comment.
If intended for main branch, should we possibly somewhere document what the feature "stats" is intended for and how it works.
There was a problem hiding this comment.
So first of all, this change is way too intrusive for what I'm guessing is the intended purpose, so a short description of what the PR intends to accomplish would be good.
Secondly, if the intent is to collect metrics from a running databroker instance, have you looked into using the tracing library (which databroker already uses for logging) in order to collect these things and wire it up to open telemetry or something in order to subscribe to it?
| { "Vehicle": { "children": { "ADAS": { "children": { "ABS": { "children": { "IsEnabled": { "datatype": "boolean", "description": "Indicates if ABS is enabled. True = Enabled. False = Disabled.", "type": "actuator", "uuid": "cad374fbfdc65df9b777508f04d5b073" }, "IsEngaged": { "datatype": "boolean", "description": "Indicates if ABS is currently regulating brake pressure. True = Engaged. False = Not Engaged.", "type": "sensor", "uuid": "6dd21979a2225e31940dc2ece1aa9a04" }, "IsError": { "datatype": "boolean", "description": "Indicates if ABS incurred an error condition. True = Error. False = No Error.", "type": "sensor", "uuid": "13cfabb3122254128234f9a696f14678" } }, "description": "Antilock Braking System signals.", "type": "branch", "uuid": "219270ef27c4531f874bbda63743b330" }, "ActiveAutonomyLevel": { "allowed": [ "SAE_0", "SAE_1", "SAE_2_DISENGAGING", "SAE_2", "SAE_3_DISENGAGING", "SAE_3", "SAE_4_DISENGAGING", "SAE_4", "SAE_5" ], "comment": "Follows https://www.sae.org/news/2019/01/sae-updates-j3016-automated-driving-graphic taxonomy. For SAE levels 3 and 4 the system is required to alert the driver before it will disengage. Level 4 systems are required to reach a safe state even if a driver does not take over. Only level 5 systems are required to not rely on a driver at all. While level 2 systems require the driver to be monitoring the system at all times, many level 2 systems, often termed \"level 2.5\" systems, do warn the driver shortly before reaching their operational limits, therefore we also support the DISENGAGING state for SAE_2.", "datatype": "string", "description": "Indicates the currently active level of autonomy according to SAE J3016 taxonomy.", "type": "sensor", "uuid": "b101c6928fc55948b1cc485e568ecd8d" }, "CruiseControl": { "children": { "IsActive": { "datatype": "boolean", "description": "Indicates if cruise control system is active (i.e. actively controls speed). True = Active. False = Inactive.", "type": "actuator", "uuid": "78ab5ce923dc5aa1a6622bcb948e1561" }, "IsEnabled": { "datatype": "boolean", "description": "Indicates if cruise control system is enabled (e.g. ready to receive configurations and settings) True = Enabled. False = Disabled |
There was a problem hiding this comment.
What changes have you done here?
| @@ -0,0 +1,16 @@ | |||
| # Use the latest version of the Rust base image | |||
There was a problem hiding this comment.
I find a bit confusing to have one dockerfile here and others in https://github.com/eclipse-kuksa/kuksa-databroker/blob/main/scripts/Dockerfile
What is the difference between them. Should better be described somewhere.
|
|
||
| --- | ||
|
|
||
| By following these steps, you will be able to build and run the Kuksa Data Broker with the stats feature enabled. If you encounter any issues or have further questions, please refer to the official documentation or seek support from the community. |
There was a problem hiding this comment.
If we are to add a stats feature we should better have a better description on what it does. Like what stats does it collect and how can you review them. I would suggest adding some screenshots - it seems that we start a grafana instance, what would you be able to see there and so on.
And this is the official documentation, isn't it?
| @@ -0,0 +1,49 @@ | |||
|
|
|||
| # Kuksa Data Broker | |||
There was a problem hiding this comment.
If we are to merge this PR I would like a link from the main README to this file
|
There are conflicts that needs to be fixed |
|
Hey @LikhithST, for now I'm closing this. As discussed we will work on a fork on this. We then can decide to open a PR with some things that we also want here. @erikbosch thanks for taking a look, without context this PR can be confusing. @LikhithST thanks for the work you have done so far. |
Adding metadata related to "time to process requests" for set and subscribe calls, adding under the feature stats