Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MQTT receiver: Add a setting to append group key to the topic name #236

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexander-akhmetov
Copy link
Contributor

@alexander-akhmetov alexander-akhmetov commented Aug 28, 2024

Adds a new setting, AddGroupKeyToTopic. If true, appends the group key to the topic name.

Grafana PR: grafana/grafana#92618

Related: grafana/grafana#16858 and #227

Copy link
Contributor

@yuri-tceretian yuri-tceretian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

n.log.Error("Failed to extract group key", "error", err.Error())
return false, fmt.Errorf("Failed to extract group key: %s", err.Error())
}
topic = path.Join(n.settings.Topic, string(groupKey))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are couple of edge cases that I think we should take care of:

  1. Let's trim / at the ends, and replace all occurrences with - or _.
  2. Same about wildcards: + and #
  3. MQTT puts restriction on length of the topic, up to 65535. Perhaps it makes sense to check it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the format of group key is like {parent_matchers[/child_matchers]+}:{group_labels} . I wonder what you think if we unwrap the matchers brackets and the topic will resemble the routing tree like.
.../parent_matchers/child_matcher/child_matcher/group_key

@alexander-akhmetov alexander-akhmetov force-pushed the alexander-akhmetov/mqtt-append-group-key-to-topic branch from 103ab89 to 568bc2c Compare September 26, 2024 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants