Skip to content

Centralize Label Prefiltering and Matching#299

Open
VJanKraemer wants to merge 5 commits intomainfrom
new_labelmatcher
Open

Centralize Label Prefiltering and Matching#299
VJanKraemer wants to merge 5 commits intomainfrom
new_labelmatcher

Conversation

@VJanKraemer
Copy link
Contributor

Fix the bug found in #294

Centralize the Prefiltering and Matching

Developer checklist (address before review)

  • Changelog.md updated
  • Prepared update for depending repositories
  • Documentation updated (public API changes only)
  • API docstrings updated (public API changes only)
  • Rebase → commit history clean
  • Squash and merge → proper PR title

* Adds a getValue function for the specified key with proper error
  handling included

Signed-off-by: Jan Kraemer <jan.kraemer@vector.com>
Signed-off-by: Jan Kraemer <jan.kraemer@vector.com>
@VJanKraemer VJanKraemer added the bug Something isn't working label Mar 9, 2026
VJanKraemer and others added 2 commits March 9, 2026 10:14
In SpecificDiscoveryStore, add the label set along with the handler info
to the DiscoveryCluster as "Controller" info. This allows us to do the
label matching centralized in the DiscoveryStore

Signed-off-by: Jan Kraemer <jan.kraemer@vector.com>
* Add test to test whether the optional label prefiltering works when
  the publishers are started before the Subscriber

Signed-off-by: Konrad Breitsprecher <Konrad.Breitsprecher@vector.com>
Co-authored-by: Jan Kraemer <jan.kraemer@vector.com>

#include "LabelMatching.hpp"
#include <optional>
#include <iostream>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#include <iostream>

baseDescriptor.SetSupplementalDataItem(supplKeyDataPublisherMediaType, "text/json");
baseDescriptor.SetSupplementalDataItem(supplKeyDataPublisherPubLabels, "- key: kA\n value: vA\n kind: 2");

ServiceDescriptor noLabelTestDescriptor{baseDescriptor};
Copy link
Member

Choose a reason for hiding this comment

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

The name noLabelTestDescriptor in this test is wrong, no? The baseDescriptor has a single label set (mandatory, key kA, value vA). I suppose this was copy-pasted way-back-then from the test above, where the baseDescriptor actually has no labels.

Signed-off-by: Marius Börschig <Marius.Boerschig@vector.com>
Co-authored-by: Jan Kraemer <jan.kraemer@vector.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants