kpetremann / mqtt-exporter

Simple generic MQTT Prometheus exporter for IoT working out of the box
https://hub.docker.com/r/kpetrem/mqtt-exporter
MIT License
100 stars 30 forks source link

Optional `/` character replacement in topic #85

Open ProtossGP32 opened 1 month ago

ProtossGP32 commented 1 month ago

Describe the solution you'd like I'd like to make the replacement of the / character in the topic label optional:

https://github.com/kpetremann/mqtt-exporter/blob/507e0925db7c39b1b84a736d51c79a80fb80d843/mqtt_exporter/main.py#L364-L367

Is there a functional reason why you always replace the / chars from the topic label? I have a use-case where I need to join time series that share at least a label with the same value as a given MQTT topic, and this makes it difficult to do it without additional post-processing steps. Also, looking at other MQTT exporters in GitHub (for example here), I don't see a problem keeping the / character in the topic value.

Could we add a boolean condition here to avoid replacing the / character? Something like this to ensure backwards compatibility:

    # parse MQTT topic
    try:
        # handle nested topic
        if not settings.TOPIC_KEEP_ORIGINAL:
            topic = topic.replace("/", "_")
    except UnicodeDecodeError:
        LOG.debug('encountered undecodable topic: "%s"', raw_topic)
        return None, None

Thanks!

Additional context

This is a use-case I'm working on. 2 time series coming from different sources shall ultimately join by the topic label:

kpetremann commented 1 month ago

Hi, I don't think there is any functional reason. It is like this since the first version, it was just for aesthetic reason. Feel free to do the PR, I'll be happy to review it.

ProtossGP32 commented 1 month ago

Hi, I don't think there is any functional reason. It is like this since the first version, it was just for aesthetic reason. Feel free to do the PR, I'll be happy to review it.

Good to know! I'll try to work on it ASAP, thanks!