feat: tedge flows statistics are published over MQTT#3843
feat: tedge flows statistics are published over MQTT#3843didier-wenzek merged 5 commits intothin-edge:mainfrom
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! 🚀 New features to boost your workflow:
|
404f0c1 to
6103e07
Compare
Robot Results
|
| let service_config = FlowsMapperConfig { | ||
| statistics_topic: Topic::new(&format!("{te}/{service_id}/statistics"))?, | ||
| status_topic: Topic::new(&format!("{te}/{service_id}/status"))?, | ||
| }; |
There was a problem hiding this comment.
It's now time to discuss the appropriate topics for flows status and statistics #3846 (comment)
I think that it would make sense to publish the statistics as measurements under {te}/{service_id}/m/statistics. But then these stats will be pushed by default to the cloud.
There was a problem hiding this comment.
Yeah publishing under the standard thin-edge.io mqtt topic structure would be more consistent, but we might need to use a different telemetry endpoint to publish this information to avoid sending too much information to the cloud by default.
We could create a convention where some local/internal information which is meant for local/on-device consumption (rather than intended for the cloud):
{te}/{service_id}/local/statisticsAlternatively, we could use flows as the type since the flows statistics maybe published on serveral services in the future, e.g. tedge-mapper-c8y, tedge-mapper-az, tedge-flows etc.
{te}/{service_id}/flows/statisticsThere was a problem hiding this comment.
A new proposal for the topic would be:
{te}/{service_id}/status/metrics
There was a problem hiding this comment.
A new proposal for the topic would be:
{te}/{service_id}/status/metrics
Done: f0f75b7
65eab46 to
9bd711c
Compare
9bd711c to
98acd21
Compare
98acd21 to
f7afd1b
Compare
4aea584 to
4419365
Compare
|
@didier-wenzek Is there an example of some of the expected stats? For testing I changed the stats interval to Below is the only message that I'm seeing (at least at the interval as set in the config) |
I found the stats I was looking for |
|
It seems that the stats are recorded on both the overall flow and individual steps. For instance, the It would be useful to be able to distinguish between the flow stats and the flow-step stats. Currently you could infer that a flow's stats has the ".toml" file in the name, however it isn't so obvious. We could just add a property the payload like "type", to determine if it is a "flow" or an onMesssage/onInterval handler, or introduce another nested topic level (e.g. |
I added a See b14d815 |
|
@didier-wenzek Nice improvements, though I would only propose one small fix to apply a minimum interval to prevent misconfiguration (1 second would be fine), as the user can set a very short interval then the stats publishing spams the mqtt broker, for example: tedge config set flows.stats.interval 0s
systemctl restart tedge-mapper-c8y
tedge mqtt sub '#' |
Done 8db18c6 |
reubenmiller
left a comment
There was a problem hiding this comment.
Approved, great persistence :)
8db18c6 to
e397633
Compare
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
The default is a statistics dump every hours Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
e397633 to
d025e0d
Compare
Proposed changes
Configure what is published, what is notTypes of changes
Paste Link to the issue
Checklist
just prepare-devonce)just formatas mentioned in CODING_GUIDELINESjust checkas mentioned in CODING_GUIDELINESFurther comments