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

Connection target topics with mixed filters and enrichment may fail to publish message #1930

Open
alstanchev opened this issue Apr 29, 2024 · 4 comments

Comments

@alstanchev
Copy link
Contributor

alstanchev commented Apr 29, 2024

If a connection target is configured with multiple topics of the same type containing different filters and enrichments. In the cases the message matches the filter of the topic that has no enrichment it will be droped.
For example:

"_/_/things/twin/events?filter=and(eq(resource:path,\"/\"),eq(topic:action,\"created\"))&extraFields=enriched_field",
"_/_/things/twin/events?filter=or(not(eq(resource:path,\"/\")),not(eq(topic:action,\"created\")))"

These two filters split the types of events in to Created and Other. It is wanted only to enrich the created events and not the others.
The flow:
When a "modify" event is received, the targets are evaluated in the signalFilter at OutboundDispatchingActor.java#L103. There the targets topics are evaluated as a whole so the modify event mathes the filter of the second topic and so the target is authorized.

Then the targets are split by having and not enrichment. So a pair is created with pair.second() only with the topic with extraFields whitout evaluating its filter, not asuming that it will not match. OutboundMappingProcessorActor#splitTargetsByExtraFields#L852

This leads for the message being droped later at OutboundMappingProcessorActor#applyFilter

Possible solutions.

  1. Evaluate again the extraFields topic filter in the filtering in splitTargetsByExtraFields.

  2. Another would be to explicitly point out in the documentation that such mixing of same topics in a single target is not permited and even maybe extend the connection config validation before creating a connection but still i think that this is a bug.

My assumption of the functionality would be that if more than one topic with filters is configured in a connection then first in presedence would be the one with extraFields that has a matching filter (because only one call for retrieving is done and we wouldn't want to multiply messages in one target) and then the others.

@thjaeckle i would like your input and thoughts on that. I was wondering if adding one more filter evaluation on each signal would not be the best approach but certenly is the smallest of a change in such a place.

@alstanchev alstanchev changed the title Connection target with topics with mixed filters and enrichment may fail to publish message Connection target topics with mixed filters and enrichment may fail to publish message Apr 29, 2024
@thjaeckle
Copy link
Member

@alstanchev I always thought that there can e.g. only be one entry for a _/_/things/twin/events in the targets of a single connection (so "kind of" a Map-semantics - however only looking at this static part, without the query params).

I would however agree that this is useful to support (having the _/_/things/twin/events several times in the targets, but with different filter and extraFields).

However, the behavior has to then clearly be defined:

  • what if several e.g. "filters" match?
    • only send out to the first matching "target"?
    • send out to all matching "targets"?
    • should this even be configurable?
  • can there be optimizations for retrieving e.g. "different" extraFields for different matching targets?
    • in order to only once fetch "combined" extraFields, instead of potentially fetching several times

@alstanchev is this a similar direction of what you thought about?

@alstanchev
Copy link
Contributor Author

alstanchev commented Apr 30, 2024

@thjaeckle

  • what if several e.g. "filters" match?
    • only send out to the first matching "target"?
    • send out to all matching "targets"? Yes
    • should this even be configurable?

I was thinking of sending to the first matching topic in a target. If more match i would consider this a configuration mistake.

  • can there be optimizations for retrieving e.g. "different" extraFields for different matching targets?
    • in order to only once fetch "combined" extraFields, instead of potentially fetching several times

From what i saw the enrichment is done once per topic in a target so i am not sure an optimisation would be necessary here. Considering that a target sends only one event, if multiple targets with matching topics are configured then they should be indipendent of one another.

@thjaeckle
Copy link
Member

Yeah, if only the first matching topic would "win", this should work.
Just needs to be documented well enough then ;)

I was wondering if adding one more filter evaluation on each signal would not be the best approach but certenly is the smallest of a change in such a place.

This should be ok, I think - should not be very "cost intense" ..

@alstanchev
Copy link
Contributor Author

Ok, will leave this for now as we have more urgent things for now and will fix when i have more time.

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

No branches or pull requests

2 participants