Skip to content

[Logstash] Move elastic_integration plugin usage to ES logstash-bridge. #131486

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

Merged

Conversation

mashhurs
Copy link
Contributor

Why we are doing this?

A bit info about plugin
[elastic_integration](plugin https://github.com/elastic/logstash-filter-elastic_integration) retrieves ingest pipelines for the data stream and runs Elastic integrations it resolved. In order to do so, It internally embeds the ES sources (ingest processors, ingest plugins, etc...), will be regularly built against ES (with all active branches) and released as a [ruby gem]https://rubygems.org/gems/logstash-filter-elastic_integration

The problem we are facing
When ES interfaces change (for e.g, GeoIP), result in plugin build breakage. So, we in the end, came up with a solution to introduce an agreement between elastic_integration plugin and ES, logstash-bridge module! The module wraps up any classes/modules the plugin depends on and whenever interface changes, developers will apply directly to the bridge Wrapper interfaces will stay without change. This will guarantee the plugin stability.

What this PR does?

It migrates majority plugin interface usages to the bridge. This investigation shows the plugin usage details.
Exports following modules to make visible in logstash-bridge:

  • ingest-user-agent (org.elasticsearch.ingest.useragent)
  • mapper-constant-keyword (org.elasticsearch.xpack.constantkeyword)
  • x-pack:plugin:redact (org.elasticsearch.xpack.redact)
  • :x-pack:plugin:spatial (only required ones: org.elasticsearch.xpack.spatial, org.elasticsearch.xpack.spatial.common and org.elasticsearch.xpack.spatial.search.aggregations.bucket.geogrid)
  • :x-pack:plugin:wildcard (org.elasticsearch.xpack.wildcard)

@elasticsearchmachine elasticsearchmachine added v9.2.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Jul 17, 2025
@mashhurs mashhurs self-assigned this Jul 18, 2025
@mashhurs mashhurs marked this pull request as ready for review July 18, 2025 20:30
@mashhurs mashhurs requested a review from a team as a code owner July 18, 2025 20:30
@elasticsearchmachine elasticsearchmachine added the needs:triage Requires assignment of a team area label label Jul 18, 2025
@mashhurs mashhurs added the Team:Core/Infra Meta label for core/infra team label Jul 18, 2025
@elasticsearchmachine elasticsearchmachine removed the Team:Core/Infra Meta label for core/infra team label Jul 18, 2025
@gbanasiak gbanasiak added the :Core/Infra/Plugins Plugin API and infrastructure label Jul 21, 2025
@elasticsearchmachine elasticsearchmachine added Team:Core/Infra Meta label for core/infra team and removed needs:triage Requires assignment of a team area label labels Jul 21, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

 - transitions terminology from wrap/unwrap to toInternal/fromInternal
 - adds abstract base class for ProcessorBridge, since we are expecting external
   implementations, which includes an internal-shaped proxy to the external
   definition.
 - adds copious commentary for the classes that were previously shipped
Copy link
Member

@yaauie yaauie left a comment

Choose a reason for hiding this comment

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

I think this looks on the right track -- I opened mashhurs#1 to refactor things a bit, and have a comment about where the constants should live.

spike refactor of logstashbridge stable API
Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

This bridge is getting quite a bit more complex, now trying to mimic some initialization of plugins. I want to reiterate and confirm something from when we first added it: this bridge is entirely an internal artifact of Logstash, right? In no way should this artifact ever be published for end users.

@mashhurs
Copy link
Contributor Author

This bridge is getting quite a bit more complex, now trying to mimic some initialization of plugins. I want to reiterate and confirm something from when we first added it: this bridge is entirely an internal artifact of Logstash, right? In no way should this artifact ever be published for end users.

👋 @rjernst, your understanding is correct that logstash-bridge is entirely internal, no end user exposure so far. From now end, https://github.com/elastic/logstash-filter-elastic_integration plugin aligns on this bridge w/o directly importing modules/classes from ES source.

This bridge is getting quite a bit more complex, now trying to mimic some initialization of plugins.

Historically, we got a customer failure (with below error) case which led us to initialize painless extensions (elastic/logstash-filter-elastic_integration#162) and this is where we need to make accessible (and open an access to its resources) [redact, wildcard, mapper-constant-keyword] modules.

logstash-1       | Caused by: java.lang.IllegalArgumentException: cannot resolve symbol [Processors]
logstash-1       |      at org.elasticsearch.painless.PainlessScript$Script.compile(long bytes = Processors.bytes(params['size']); ctx.size_in_bytes = bytes;:24) ~[?:?]
logstash-1       |      at org.elasticsearch.painless.phase.DefaultSemanticAnalysisPhase.visitCall(DefaultSemanticAnalysisPhase.java:3233) 

Thank you for taking a look. We believe this modular descriptor change is beyond logstash-team scope. If you prefer to make this modular change with a separate PR (to get reviewed from corresponded team), I will make it happen but if your concern is about non-qualified opens statements, I have updated them to be specific (thank you for catching, I added it to spatial but somehow forgot adding to others).

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

Thanks @mashhurs for modularizing these. The change looks fine, just one last ask on the module names.

mashhurs and others added 3 commits July 22, 2025 23:02
…y that remove xpack from namings. Move constants from processor bridge to align with a proper place where Ingest common plugin bridge is sutable.
Copy link
Member

@yaauie yaauie left a comment

Choose a reason for hiding this comment

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

Fantastic work. Thank you.

@mashhurs mashhurs merged commit 1e87f67 into elastic:main Jul 24, 2025
33 checks passed
@mashhurs mashhurs deleted the move-to-bridge-stable-api-investigation branch July 24, 2025 16:13
szybia added a commit to szybia/elasticsearch that referenced this pull request Jul 25, 2025
…king

* upstream/main: (90 commits)
  Register a blob cache long counter metric for total evicted regions (elastic#131862)
  Move plan attribute resolution to its own component (elastic#131830)
  Make restore support multi-project (elastic#131661)
  Use logically more correct expression (elastic#131869)
  [ES|QL] Change equals and hashcode for ConstantNullBlock (elastic#131817)
  Update `TransportVersion` to support a new model (elastic#131488)
  Correct slow log user for RCS 2.0 (elastic#130140)
  Revert "Remove 8.17 from dev branches"
  Mute org.elasticsearch.compute.aggregation.ValuesBytesRefGroupingAggregatorFunctionTests testSomeFiltered elastic#131878
  Remove 8.17 from dev branches
  Revert "CompressorFactory.compressor (elastic#131655)" (elastic#131866)
  Add fast path for single value in VALUES aggregator (elastic#130510)
  Resolve inference release tests failing due to missing feature flag  (elastic#131841)
  [Docs] Replace placeholder URLs (elastic#131309)
  CompressorFactory.compressor (elastic#131655)
  add availability info for speed loading setting (elastic#131714)
  [Logstash] Move `elastic_integration` plugin usage to ES logstash-bridge. (elastic#131486)
  Migrate x-pack-enrich legacy rest tests to new test framework (elastic#131743)
  Fix plugin example test failures due to deprecation warning (elastic#131819)
  Remove deprecated function isNotNullAndFoldable (elastic#130944)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Plugins Plugin API and infrastructure external-contributor Pull request authored by a developer outside the Elasticsearch team >feature Team:Core/Infra Meta label for core/infra team v9.2.0
Projects
None yet
5 participants