Improved patterns command with new algorithm by songkant-aws · Pull Request #3263 · opensearch-project/sql

songkant-aws · 2025-01-24T14:49:46Z

Description

This PR introduces enhancement on original patterns command. It keeps the patterns command naming and rebuild it with specific window functions to parse log messages with different pattern method(log parser algorithms). See this RFC: #3251

The sample query input and output will be like:

New brain algorithm log parser:

Original regex based patterns command:

After changing default pattern method to brain, the default behavior is changed:

Related Issues

Resolves #3251

Check List

New functionality includes testing.
New functionality has been documented.
New functionality has javadoc added.
New functionality has a user manual doc added.
API changes companion pull request created.
Commits are signed per the DCO using --signoff.
Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Songkan Tang <songkant@amazon.com>

YANG-DB · 2025-01-24T20:07:24Z

Thanks for the initiative!!

penghuo

In general,

add user facing doc to explain new BRAIN alg with example will be helpfull.
add UT could help reviwer understand the logic also. @dai-chen did a good example. https://github.com/opensearch-project/sql/blob/main/core/src/main/java/org/opensearch/sql/expression/window/frame/PeerRowsWindowFrame.java
add benchmark to compare BRAIN and SIMPLE also help a lot.

penghuo · 2025-01-25T01:06:41Z

ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java

+                new Function(
+                    ctx.pattern_method != null
+                        ? ctx.pattern_method.getText().toLowerCase(Locale.ROOT)
+                        : BuiltinFunctionName.BRAIN.name(), // By default, use new algorithm


Could we introduce a setting to control the default pattern algorithms?

Ok, I see. Like a java property to specify default algorithm name? I can add that.

penghuo · 2025-01-25T01:35:19Z

...src/main/java/org/opensearch/sql/expression/window/patterns/StreamPatternWindowFunction.java

+                    ((NamedArgumentExpression) expression).getValue().valueOf().stringValue())
+            .findFirst()
+            .orElse("");
+    return new StreamPatternRowWindowFrame(


do we need StreamPatternRowWindowFrame? could we just use CurrentRowWindowFrame instead? patternExpression could be member of StreamPatternWindowFunction?

Sure, I will remove the need of StreamPatternRowWindowFrame in the next revision.

penghuo · 2025-01-25T01:50:54Z

core/src/main/java/org/opensearch/sql/expression/window/frame/PeerRowsWindowFrame.java

  }

-  private boolean isSamePartition(ExprValue next) {
+  protected boolean isSamePartition(ExprValue next) {


revert unnecessary change.

penghuo · 2025-01-25T01:54:52Z

core/src/main/java/org/opensearch/sql/expression/window/frame/BufferPatternRowsWindowFrame.java

+                  return value.stringValue();
+                })
+            .toList();
+    this.preprocessedMessages.addAll(logParser.preprocessAllLogs(logMessages));


BufferPatternRowsWindowFrame should have it own spec, doet it means over all rows? @dai-chen WindowFrame definition and How to use WIndowFrame should be seperate, right?

For simplicity, I set the spec to empty partition by and empty sort by during AST tree parsing unresolved WindowFunction, which treats the window frame range is all rows on coordinator node. Because I haven't seen requirements on sorting and partitioning on other columns.

I think we can add partition by and sort by syntax if we see values in case users want to specify them. Thoughts?

Yes, I think users are free to use pattern functions added with any window frame definition.

Tests with non-empty partition by and non-empty sort by are still required in PR.

Added non-empty partition by and sort by unit tests.

Signed-off-by: Songkan Tang <songkant@amazon.com>

dai-chen · 2025-01-27T22:10:50Z

core/src/main/java/org/opensearch/sql/expression/window/frame/BufferPatternRowsWindowFrame.java

+                  return value.stringValue();
+                })
+            .toList();
+    this.preprocessedMessages.addAll(logParser.preprocessAllLogs(logMessages));


Yes, I think users are free to use pattern functions added with any window frame definition.

dai-chen · 2025-01-27T23:18:54Z

core/src/main/java/org/opensearch/sql/expression/window/WindowFunctions.java

+    repository.register(brain());
+    repository.register(simplePattern());


I'm thinking are these new algorithm really window function? If users specify order by or partition by, does it still generate meaningful result?

dai-chen · 2025-01-27T23:20:25Z

core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java

+      UnresolvedExpression sourceField,
+      String alias,
+      java.util.Map<String, Literal> arguments) {
+    return new Pattern(


Just wonder if pattern is mostly syntax sugar for pattern window function, is new logical operator still required?

After the integration with LogicalWindow, I see they are quite similar. Yeah, I think Pattern operator is probably not needed.

I don't find the Window unresolvedPlan. So replace Pattern with Window instead.

dai-chen · 2025-01-27T23:42:32Z

...src/main/java/org/opensearch/sql/expression/window/patterns/BufferPatternWindowFunction.java

+import org.opensearch.sql.expression.window.frame.WindowFrame;
+
+@EqualsAndHashCode(callSuper = true)
+public class BufferPatternWindowFunction extends FunctionExpression


order by expression in window definition decides who're peers right? What's the order by expression for buffer (offline) pattern function? I didn't find it and may miss it in design doc:

Project[message#1, patterns_field#2] +- Window[brain(message#1), windowsSpec(partitionBy=null, frame=PeerRowsWindowFrame)] +- OpenSearchIndexScan

dai-chen

A high-level question about the long-term approach: Currently, the pattern command always generates a new field, which offers flexibility as users can perform aggregation (stats) or other operations on it afterward. However, for large datasets, I assume most users would prefer functionality similar to CloudWatch Logs Query Syntax - Pattern.

Do you see any potential issues with the current implementation, which is based on SQL window functions combined with aggregate functions, as we scale this feature in the future?

songkant-aws · 2025-01-28T01:57:34Z

Do you see any potential issues with the current implementation, which is based on SQL window functions combined with aggregate functions, as we scale this feature in the future?

@dai-chen It's a good question. Actually, it's one of my previous thought to use a new specific operator to directly return grouped logs with sample count or grouped samples.

It's more like a compound aggregation operator with a default group by. But current aggregation abstraction only supports row by row iteration. The log pattern algorithms cover both streaming and buffering computing paradigm. Then I figure out this is more close to window operator's abstraction. To achieve what Cloudwatch has done, I think we could extend pattern functions from AggregateWindowFunction that has a special aggregator state structure.

As to partition by and sort by specification, my initial plan is to deliver a simple version without them because that's probably enough.
partition by is probably more useful than sort by. Imagine users may want to see different log patterns for INFO, WARN, ERROR partitions.
I haven't seen a use case for sort by yet. Maybe they need to see latest logs sort by date?

songkant-aws · 2025-01-28T02:10:17Z

To scale the pattern function, I think other issues are pending investigation.

How to push down operator to OpenSearch data node? Is Painless script one of option?
OpenSearch doesn't have shuffle mechanism for now, how to perform large dataset aggregation like operation with simple map reduce? If data volume is quite large, spilling additional data to disk is probably required during execution.
To log pattern, streaming algorithms are probably more suitable to large scale dataset. We may need to introduce more by then.

LantaoJin · 2025-02-10T09:35:37Z

core/src/main/java/org/opensearch/sql/expression/window/frame/BufferPatternRowsWindowFrame.java

+                  return value.stringValue();
+                })
+            .toList();
+    this.preprocessedMessages.addAll(logParser.preprocessAllLogs(logMessages));


Tests with non-empty partition by and non-empty sort by are still required in PR.

LantaoJin · 2025-02-10T09:44:17Z

docs/user/ppl/cmd/patterns.rst

+
+Simple Pattern
+============
+patterns [new_field=<new-field-name>] [pattern=<pattern>] <field> SIMPLE_PATTERN


The seems a breaking change, why not make SIMPLE_PATTERN optional for compatibility.

It's still compatible. If we omit SIMPLE_PATTERN, it will use BRAIN method by default. Peng suggested we can add a configuration to decide which is the default behavior.

It's still compatible. If we omit SIMPLE_PATTERN, it will use BRAIN method by default. Peng suggested we can add a configuration to decide which is the default behavior.

We don't call it compatible if it brings breaking change with original command syntax. I doubt about using the new BRAIN algorithm as default output before it has been running stably for a while. @penghuo any thoughts here?

Peng suggested we can add a configuration to decide which is the default behavior.

Did we add setting already?
If not, we should default to SIMPLE_PATTERN.

Add a new setting now. It will default to SIMPLE_PATTERN. Please review the change. Let me know if it makes sense.

LantaoJin · 2025-02-10T09:46:03Z

ppl/src/main/antlr/OpenSearchPPLParser.g4

+patternMethod
+   : SIMPLE_PATTERN
+   | BRAIN


patternMethod should be added to keywordsCanBeId as well.

LantaoJin · 2025-02-10T09:46:38Z

ppl/src/main/antlr/OpenSearchPPLParser.g4

+patternsMethod
+   : PUNCT
+   | REGEX


patternsMethod should be added to keywordsCanBeId

penghuo · 2025-02-13T03:59:56Z

@songkant-aws could u resolve conflict?

Signed-off-by: Songkan Tang <songkant@amazon.com>

songkant-aws · 2025-02-14T06:31:21Z

@penghuo @dai-chen @LantaoJin I have addressed several comments and added a new setting plugins.ppl.default.pattern.method to allow overriding default pattern method. For now, the default pattern method is still regex based SIMPLE_PATTERN. Please let me know if the current change is reasonable. Thanks!

penghuo · 2025-02-14T16:10:45Z

@songkant-aws please fix failed UT coverage. then we are ready to merge.

Signed-off-by: Songkan Tang <songkant@amazon.com>

songkant-aws · 2025-02-17T12:10:12Z

@penghuo Fixed all of CI checks with more test coverage and cases.

penghuo

Thx!

LantaoJin · 2025-02-18T09:01:54Z

ppl/src/main/java/org/opensearch/sql/ppl/parser/AstBuilder.java

-    ImmutableMap.Builder<String, Literal> builder = ImmutableMap.builder();
+    List<UnresolvedExpression> unresolvedArguments = new ArrayList<>();
+    unresolvedArguments.add(sourceField);
+    AtomicReference<String> alias = new AtomicReference<>("patterns_field");


AstBuilder is created per query. No necessary to wrap with AtomicReference

Seems like IDE has compilation error when using a temp String variable in lambda expression. So I use AtomicReference as a workaround

Ok, I see, the String alias must be a "final" variable from the enclosing scope.

LantaoJin · 2025-02-18T09:09:18Z

common/src/main/java/org/opensearch/sql/common/patterns/BrainLogParser.java

+    if (thresholdPercentage < 0.0f || thresholdPercentage > 1.0f) {
+      throw new IllegalArgumentException("Threshold percentage must be between 0.0 and 1.0");
+    }


What if the thresholdPercentage = 0 or thresholdPercentage = 1

It's valid, which means 0 or the highest frequency.

LantaoJin · 2025-02-18T09:23:47Z

...src/main/java/org/opensearch/sql/expression/window/patterns/BufferPatternWindowFunction.java

+import org.opensearch.sql.expression.window.frame.WindowFrame;
+import org.opensearch.sql.utils.FunctionUtils;
+
+@EqualsAndHashCode(callSuper = true)


minor: @ToString

I override this function's toString specifically.

LantaoJin · 2025-02-18T09:24:13Z

...src/main/java/org/opensearch/sql/expression/window/patterns/StreamPatternWindowFunction.java

+import org.opensearch.sql.expression.window.frame.WindowFrame;
+import org.opensearch.sql.utils.FunctionUtils;
+
+@EqualsAndHashCode(callSuper = true)


LantaoJin

No blocker any more.

opensearch-trigger-bot · 2025-02-18T10:09:57Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/sql/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/sql/backport-2.x
# Create a new branch
git switch --create backport/backport-3263-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 44ff520f08b606257240dd009758788638f24acb
# Push it to GitHub
git push --set-upstream origin backport/backport-3263-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/sql/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3263-to-2.x.

* Improved patterns command with new algorithm Signed-off-by: Songkan Tang <songkant@amazon.com> * Minor change log parser default configurations Signed-off-by: Songkan Tang <songkant@amazon.com> * Refactor a bit and add partial unit tests Signed-off-by: Songkan Tang <songkant@amazon.com> * Add more unit tests Signed-off-by: Songkan Tang <songkant@amazon.com> * Amend patterns user facing doc Signed-off-by: Songkan Tang <songkant@amazon.com> * Add average time benchmark for patterns window functions Signed-off-by: Songkan Tang <songkant@amazon.com> * Add new default pattern method setting to allow change Signed-off-by: Songkan Tang <songkant@amazon.com> * Update unit tests per injected setting Signed-off-by: Songkan Tang <songkant@amazon.com> * Update patterns command user facing doc after introducing setting Signed-off-by: Songkan Tang <songkant@amazon.com> * Complement more unit test cases Signed-off-by: Songkan Tang <songkant@amazon.com> * Adjust patterns.rst file format Signed-off-by: Songkan Tang <songkant@amazon.com> * Handle null like ExprValue cases and fix additional doctest Signed-off-by: Songkan Tang <songkant@amazon.com> * Fix spotless style check Signed-off-by: Songkan Tang <songkant@amazon.com> * Minor doctest fix style Signed-off-by: Songkan Tang <songkant@amazon.com> --------- Signed-off-by: Songkan Tang <songkant@amazon.com>

Signed-off-by: Songkan Tang <songkant@amazon.com>

Signed-off-by: Songkan Tang <songkant@amazon.com> Signed-off-by: xinyual <xinyual@amazon.com>

Improved patterns command with new algorithm

9d93991

Signed-off-by: Songkan Tang <songkant@amazon.com>

songkant-aws requested review from GumpacG, LantaoJin, MaxKsyunz, Swiddis, YANG-DB, Yury-Fridlyand, acarbonetto, anirudha, dai-chen, derek-ho, forestmvey, joshuali925, kavithacm, mengweieric, noCharger, penghuo, ps48, seankao-az and ykmr1224 as code owners January 24, 2025 14:49

penghuo reviewed Jan 25, 2025

View reviewed changes

penghuo added the enhancement New feature or request label Jan 25, 2025

songkant-aws added 2 commits January 26, 2025 16:29

Minor change log parser default configurations

6874a5f

Signed-off-by: Songkan Tang <songkant@amazon.com>

Merge branch 'main' into log-pattern-command

dd9373f

dai-chen reviewed Jan 27, 2025

View reviewed changes

LantaoJin requested changes Feb 10, 2025

View reviewed changes

songkant-aws added 4 commits February 13, 2025 15:27

Merge branch 'main' into log-pattern-command

0555813

Signed-off-by: Songkan Tang <songkant@amazon.com>

Add new default pattern method setting to allow change

3b6245f

Signed-off-by: Songkan Tang <songkant@amazon.com>

Update unit tests per injected setting

b86f132

Signed-off-by: Songkan Tang <songkant@amazon.com>

Update patterns command user facing doc after introducing setting

43f0c62

Signed-off-by: Songkan Tang <songkant@amazon.com>

songkant-aws added 5 commits February 17, 2025 14:07

Complement more unit test cases

b19e9ae

Signed-off-by: Songkan Tang <songkant@amazon.com>

Adjust patterns.rst file format

7fc03a0

Signed-off-by: Songkan Tang <songkant@amazon.com>

Handle null like ExprValue cases and fix additional doctest

df16d64

Signed-off-by: Songkan Tang <songkant@amazon.com>

Fix spotless style check

a15fcd6

Signed-off-by: Songkan Tang <songkant@amazon.com>

Minor doctest fix style

bfec862

Signed-off-by: Songkan Tang <songkant@amazon.com>

penghuo approved these changes Feb 17, 2025

View reviewed changes

penghuo added the backport 2.x label Feb 17, 2025

songkant-aws requested a review from LantaoJin February 18, 2025 09:19

LantaoJin reviewed Feb 18, 2025

View reviewed changes

LantaoJin approved these changes Feb 18, 2025

View reviewed changes

LantaoJin merged commit 44ff520 into opensearch-project:main Feb 18, 2025
16 checks passed

opensearch-trigger-bot bot added the backport-failed label Feb 18, 2025

songkant-aws mentioned this pull request Feb 19, 2025

Backport/backport 3263 to 2.x #3335

Merged

7 tasks

penghuo pushed a commit that referenced this pull request Feb 19, 2025

Improved patterns command with new algorithm (#3263) (#3335)

0d74995

Signed-off-by: Songkan Tang <songkant@amazon.com>

LantaoJin mentioned this pull request Apr 29, 2025

Support eventstats command with Calcite #3585

Merged

7 tasks

penghuo pushed a commit that referenced this pull request Jun 16, 2025

Improved patterns command with new algorithm (#3263) (#3335)

0dc4e02

Signed-off-by: Songkan Tang <songkant@amazon.com> Signed-off-by: xinyual <xinyual@amazon.com>

songkant-aws deleted the log-pattern-command branch October 24, 2025 07:30

qianheng-aws mentioned this pull request Mar 18, 2026

Add songkant-aws as maintainer #5244

Open

		repository.register(brain());
		repository.register(simplePattern());

Conversation

songkant-aws commented Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issues

Check List

Uh oh!

YANG-DB commented Jan 24, 2025

Uh oh!

penghuo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

songkant-aws Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dai-chen Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dai-chen Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dai-chen left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

songkant-aws commented Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

songkant-aws commented Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

songkant-aws Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

penghuo Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

songkant-aws commented Jan 24, 2025 •

edited

Loading

songkant-aws Jan 27, 2025 •

edited

Loading

dai-chen Jan 27, 2025 •

edited

Loading

dai-chen Jan 27, 2025 •

edited

Loading

dai-chen left a comment •

edited

Loading

songkant-aws commented Jan 28, 2025 •

edited

Loading

songkant-aws commented Jan 28, 2025 •

edited

Loading

songkant-aws Feb 10, 2025 •

edited

Loading

penghuo Feb 13, 2025 •

edited

Loading

penghuo commented Feb 13, 2025 •

edited

Loading