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

[ISSUE #4577] Implement FilterEngine for EventMesh Filters #4578

Merged
merged 16 commits into from Nov 28, 2023

Conversation

xwm1992
Copy link
Contributor

@xwm1992 xwm1992 commented Nov 24, 2023

Fixes #4577 .

Motivation

Explain the content here.
Explain why you want to make the changes and what problem you're trying to solve.

Modifications

Describe the modifications you've done.

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

Copy link

codecov bot commented Nov 24, 2023

Codecov Report

Attention: 208 lines in your changes are missing coverage. Please review.

Comparison is base (4e72b3c) 16.95% compared to head (8e14a5b) 16.78%.

Files Patch % Lines
...rg/apache/eventmesh/runtime/boot/FilterEngine.java 0.00% 55 Missing ⚠️
...eventmesh/meta/nacos/service/NacosMetaService.java 0.00% 48 Missing ⚠️
...ics/prometheus/metrics/PrometheusHttpExporter.java 0.00% 35 Missing ⚠️
...otocol/http/processor/SendAsyncEventProcessor.java 0.00% 28 Missing ⚠️
...rics/prometheus/metrics/PrometheusTcpExporter.java 0.00% 11 Missing ⚠️
...ics/prometheus/metrics/PrometheusGrpcExporter.java 0.00% 8 Missing ⚠️
.../core/protocol/http/push/AsyncHTTPPushRequest.java 0.00% 8 Missing ⚠️
...he/eventmesh/runtime/boot/EventMeshHTTPServer.java 0.00% 5 Missing ⚠️
...org/apache/eventmesh/runtime/meta/MetaStorage.java 0.00% 5 Missing ⚠️
...java/org/apache/eventmesh/filter/PatternEntry.java 66.66% 1 Missing ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4578      +/-   ##
============================================
- Coverage     16.95%   16.78%   -0.17%     
+ Complexity     1678     1675       -3     
============================================
  Files           781      782       +1     
  Lines         29167    29293     +126     
  Branches       2510     2527      +17     
============================================
- Hits           4945     4918      -27     
- Misses        23761    23916     +155     
+ Partials        461      459       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@pandaapo pandaapo left a comment

Choose a reason for hiding this comment

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

parseRequiredField(key, "$." + key, value, pattern);
if (value.isEmpty()) {
// Empty array
throw new JsonException("INVALID_PATTERN_VALUE ");
}
PatternEntry patternEntry = new PatternEntry(key, "$." + key);
for (final JsonNode objNode : value) {
// {
// "suffix":".jpg"
// }
Condition condition = parseCondition(objNode);
patternEntry.addCondition(condition);
}
pattern.addRequiredFieldList(patternEntry);

I'm not sure if I'm misunderstanding something, but I think there's a duplicate PatternEntry in the Pattern. For example, the key is "source" and the value is the following array.

String condition = "{
    "source":[
        {
            "prefix":"eventmesh."
        },
				{
            "suffix":"eventmesh."
        },
    ]
}";

In line 77, a PatternEntry is created. In lines 82-89, another PatternEntry is created that is identical to the first one. Both are added to the requiredFieldList of the Pattern.


上面这段代码我不确定是不是我哪里理解错了,感觉在Pattern里面构建了重复的PatternEntry。我以以下规则为例,key是"source",value是后面的数组。

String condition = "{
    "source":[
        {
            "prefix":"eventmesh."
        },
				{
            "suffix":"eventmesh."
        },
    ]
}";

第77行代码,构建了一次PatternEntry。第82-89行代码,又构建了一次和前面一样的PatternEntry,都添加到了Pattern的requiredFieldList中。

metaServiceListener = this::updateFilterPatternMap;

// addListeners for producerManager & consumerManager
scheduledExecutorService.scheduleAtFixedRate(() -> {
Copy link
Contributor

@pandaapo pandaapo Nov 24, 2023

Choose a reason for hiding this comment

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

Could you add another task to pull the rule configuration from the remote configuration center, so that we can obtain the new rules?

这里面是不是还可以再增加一个任务,去拉取远程配置中心的规则配置,这样可以获取新增的规则?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The timing task is currently configured based on the groups of the producer manager and the consumer manager. Because the remote configuration center needs to be configured based on the group, the actual rule effectiveness also depends on whether these groups exist. It only applies to groups that exist in the mesh. The monitoring function of configured rules is implemented, and the active pull task is not performed. The active pull can only perform a fuzzy matching rule pull after starting the mesh.


这个定时任务目前是根据producerManager与consumerManager的group进行的配置监听,因为远程配置中心需要根据group来配置,实际上规则生效也取决于是否存在这些group,因此只对mesh中存在的group做了规则配置的监听功能,没有做主动拉取任务,主动拉取只会在启动mesh后进行一次模糊匹配的规则拉取

Copy link
Contributor

@pandaapo pandaapo Nov 26, 2023

Choose a reason for hiding this comment

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

This way, dynamic changes with existing rules can be perceived during runtime. If a new group is added along with its corresponding rules, it requires a restart by executing FilterEngine#start(). Similarly, if an existing group initially lacks configured rules, and rules for that group are added during runtime, a restart is also necessary.

这样运行中能感知既有规则的动态变更。但如果新增了group并相应地新增其规则就需要重启执行FilterEngine#start()了。或者某既有group先没有配置规则,然后运行中增加该group的规则,也需要重启才行。

handlerSpecific.sendResponse(responseHeaderMap, responseBodyMap);
}
if (isFiltered) {
eventMeshProducer.send(sendMessageContext, new SendCallback() {
Copy link
Contributor

@pandaapo pandaapo Nov 24, 2023

Choose a reason for hiding this comment

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

I don't quite understand this point. Is it necessary for producers to set filtering rules when sending messages?

这一点没太明白,生产者发送消息有必要设置过滤规则吗?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, the settings of filtering rules are retained to ensure that if there is a need for the sender to use filtering rules in the future.


目前是都保留了过滤规则的设置,确保后续如果有发送方的使用过滤规则的需求。

key = key + "*";
}
int pageNo = 1;
int pageSize = 100;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can pageNo and pageSize be parameterized to make it easier to use for eventmesh-admin calls in the future?

pageNopageSize是不是可以参数化,方便后续eventmesh-admin调用时使用?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These two parameters are currently only used for fuzzy queries in nacos. I am not sure whether they will be used for other components, so I have not made any modifications at the api layer to supplement these two parameters. If they are used in other extended implementations later If needed, the parameters can be extended to the api layer.


这两个参数目前只是nacos中的模糊查询时有用到,其他的组件我还不确定是否会用到,因此我没有在api层做修改补充这两个参数,如果后续在其他的扩展实现中也需要用到,可以将参数扩展到api层。

Copy link
Contributor

@pandaapo pandaapo left a comment

Choose a reason for hiding this comment

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

Can a markdown document be written to explain the writing of filtering rules? Because filtering rules have requirements for keywords.

是不是可以写个markdown文档来说明下过滤规则的书写?因为过滤规则对关键字是有要求的。

@xwm1992
Copy link
Contributor Author

xwm1992 commented Nov 26, 2023

parseRequiredField(key, "$." + key, value, pattern);
if (value.isEmpty()) {
// Empty array
throw new JsonException("INVALID_PATTERN_VALUE ");
}
PatternEntry patternEntry = new PatternEntry(key, "$." + key);
for (final JsonNode objNode : value) {
// {
// "suffix":".jpg"
// }
Condition condition = parseCondition(objNode);
patternEntry.addCondition(condition);
}
pattern.addRequiredFieldList(patternEntry);

I'm not sure if I'm misunderstanding something, but I think there's a duplicate PatternEntry in the Pattern. For example, the key is "source" and the value is the following array.

String condition = "{
    "source":[
        {
            "prefix":"eventmesh."
        },
				{
            "suffix":"eventmesh."
        },
    ]
}";

In line 77, a PatternEntry is created. In lines 82-89, another PatternEntry is created that is identical to the first one. Both are added to the requiredFieldList of the Pattern.

上面这段代码我不确定是不是我哪里理解错了,感觉在Pattern里面构建了重复的PatternEntry。我以以下规则为例,key是"source",value是后面的数组。

String condition = "{
    "source":[
        {
            "prefix":"eventmesh."
        },
				{
            "suffix":"eventmesh."
        },
    ]
}";

第77行代码,构建了一次PatternEntry。第82-89行代码,又构建了一次和前面一样的PatternEntry,都添加到了Pattern的requiredFieldList中。

yes, you are right, I'll fix it.

@xwm1992
Copy link
Contributor Author

xwm1992 commented Nov 26, 2023

Can a markdown document be written to explain the writing of filtering rules? Because filtering rules have requirements for keywords.

是不是可以写个markdown文档来说明下过滤规则的书写?因为过滤规则对关键字是有要求的。

会的,filter和transformer都实现后会补充各自的文档

@xwm1992 xwm1992 merged commit 2fcd091 into apache:master Nov 28, 2023
10 of 13 checks passed
wizardzhang pushed a commit to wizardzhang/eventmesh that referenced this pull request Dec 1, 2023
…che#4578)

* update ci.yml

* add filters

* add filters

* add filters

* apply filters under http processor

* update filterEngine

* [ISSUE apache#4577] Implement FilterEngine for EventMesh Filters

* refactor ci.yml

* fix ci check error

* fix ci check error

* fix ci check error

* fix ci check error

* remove duplicate method in PatternBuilder
@pandaapo pandaapo added this to the 1.10 milestone Dec 5, 2023
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.

[Feature] Implement FilterEngine for EventMesh Filters
4 participants