atlas: fix invalid tag characters at meter creation - #1268
Conversation
Move the replacement of characters not permitted by the storage layer from the report paths to meter creation. Previously the fix was applied independently on the publish path (MeasurementSerializer) and the LWC streaming path (IdMapper), which allowed the two to diverge: subscription matching on the streaming path ran against the raw id, so a query written against the sanitized form (e.g. ipc.endpoint,_api_v,:re) would match the persisted data but silently drop on streaming. Add a protected AbstractRegistry.normalizeTags hook (default no-op) that runs for every id used to create or look up a meter, and override it in AtlasRegistry to replace invalid characters using the configured validTagCharacters set. Meters are now created with a valid id, so both report paths receive already-fixed ids and no longer need to fix them. This also makes ids that differ only by invalid characters collapse to a single meter/series. - normalizeTags returns the same id instance when nothing needs fixing (uses AsciiSet.containsAll to detect without allocating) and only rebuilds via Id.unsafeCreate when a replacement is required. A null validTagCharacters short circuits with no work. - Remove the now-redundant fixing from MeasurementSerializer, JsonUtils, DefaultPublisher, and the EvaluatorConfig idMapper; drop the vestigial fixTagString parameter from IdMapper. - Cache normalizeTags inside idNormalizationCache for foreign ids so the fully-normalized id is cached; DefaultId keeps the type short circuit. - AsciiSet.containsAll: scan directly instead of comparing indexOfNonMember to length twice.
manolama
left a comment
manolama
left a comment
There was a problem hiding this comment.
It makes sense but it will impact performance for the ID creation where folks aren't memoizing the ID (the majority of cases I assume). It would further impact anything that requires replacement like IPC metrics with endpoints as the rewrite will happen on each update.
| return true; | ||
| } | ||
|
|
||
| private int indexOfNonMember(CharSequence str) { |
There was a problem hiding this comment.
Worth deleting if we aren't using it any more?
There was a problem hiding this comment.
It is still used in other places, it was removed from containsAll to avoid the extra length check in the hot path.
| * @param fixTagString | ||
| * Function that fixes characters used for tag keys and values. | ||
| */ | ||
| public MeasurementSerializer(Function<String, String> fixTagString) { |
There was a problem hiding this comment.
This will break anyone that needs different formatting for a destination, but no one is doing that internally and likely no one externally.
There was a problem hiding this comment.
It is unlikely any of those would just care about string formatting. Typically they would implement the publisher.
2 participants
Move the replacement of characters not permitted by the storage layer from the report paths to meter creation. Previously the fix was applied independently on the publish path (MeasurementSerializer) and the LWC streaming path (IdMapper), which allowed the two to diverge: subscription matching on the streaming path ran against the raw id, so a query written against the sanitized form (e.g. ipc.endpoint,_api_v,:re) would match the persisted data but silently drop on streaming.
Add a protected AbstractRegistry.normalizeTags hook (default no-op) that runs for every id used to create or look up a meter, and override it in AtlasRegistry to replace invalid characters using the configured validTagCharacters set. Meters are now created with a valid id, so both report paths receive already-fixed ids and no longer need to fix them. This also makes ids that differ only by invalid characters collapse to a single meter/series.