feat: emit mqtt metrics #106
Conversation
integration/src/main/java/com/aws/greengrass/mqtt/moquette/ClientDeviceAuthorizer.java
Outdated
Show resolved
Hide resolved
integration/src/main/java/com/aws/greengrass/mqtt/moquette/ClientDeviceAuthorizer.java
Outdated
Show resolved
Hide resolved
| break; | ||
| } | ||
| metricsEmitter.emitMetric(Metric.builder() | ||
| .namespace(MqttMetrics.MOQUETTE_MQTT_NAMESPACE) |
There was a problem hiding this comment.
let's talk to cloud team about the namespaces. We may want to share a namespace for any MQTT broker.
There was a problem hiding this comment.
IoT Core Message broker metrics are under "MQTT" Protocol dimension in "AWS/IoT" namespace. Does it make sense to keep generic "LocalMQTT" namespace housing metrics for our local-mqtt broker?
| authErrorMetric = MqttMetrics.UNKNOWN_AUTH_ERROR; | ||
| break; | ||
| } | ||
| metricsEmitter.emitMetric(Metric.builder() |
There was a problem hiding this comment.
we probably do not want to immediately emit a metric, this can be expensive. We may want to instead batch up a number of them and then emit the sum ourselves on a timer.
There was a problem hiding this comment.
updated to schedule batched metric emit
|
Unit Tests Coverage Report
Minimum allowed coverage is Generated by 🐒 cobertura-action against 2df709a |
| import javax.inject.Inject; | ||
|
|
||
| public class MqttMetricsEmitter { | ||
| private static final long DEFAULT_METRIC_EMIT_FREQUENCY_SECONDS = 30; |
There was a problem hiding this comment.
Should we make this configurable?
Description of changes:
How was this change tested:
mvn clean packagewith unit tests.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.