Skip to content

adds LogStash::PluginMetadata for simple key/value plugin metadata#10636

Closed
yaauie wants to merge 1 commit intoelastic:masterfrom
yaauie:plugin-metadata-registry
Closed

adds LogStash::PluginMetadata for simple key/value plugin metadata#10636
yaauie wants to merge 1 commit intoelastic:masterfrom
yaauie:plugin-metadata-registry

Conversation

@yaauie
Copy link
Copy Markdown
Member

@yaauie yaauie commented Apr 4, 2019

We need a way for a plugin to register simple metadata about external
resources it connects to in order to implement a Monitoring feature in which
an Elasticsearch Output Plugin can store the connected cluster's uuid (#10602)

Here, we add a generic LogStash::PluginMetadata along with a registry, and
expose an accessor on LogStash::Plugin#plugin_metadata so that instances
can access their own metadata object.


Implementation notes: I would like to reimplement the backing store to use a
Java implementation with interned strings, and replace this implementation with
a jruby wrapper, which would allow us to find a way to shim this into the Java API's
Context; as the current requirements do not encompass the Java API, I am
submitting this PR as-is with only support for the Ruby API and deferring
further discussion after getting teammates unblocked.

We need a way for a plugin to register simple metadata about external
resources it connects to in order to implement a Monitoring feature in which
an Elasticsearch Output Plugin can store the connected cluster's uuid (elastic#10602)

Here, we add a generic `LogStash::PluginMetadata` along with a registry, and
expose an accessor on `LogStash::Plugin#plugin_metadata` so that instances
can access their own metadata object.
Copy link
Copy Markdown
Contributor

@danhermann danhermann left a comment

Choose a reason for hiding this comment

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

I'm +1 to this as a mechanism for plugins to publish arbitrary metadata about themselves. I presume it would be exposed through the pipeline stats API or the like?

I do think that it's important that this be available for both Ruby and Java plugins since Java plugins are planned for both GA and functional parity with Ruby plugins in 7.1.

#
# Data is persisted across pipeline reloads, and no effort is made to clean up metadata from pipelines
# that no longer exist after a pipeline reload.
#
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would think it potentially problematic if data were persisted across pipeline reloads if each plugin's data is keyed by the plugin's ID. It's quite common for plugin ID to be auto-generated GUIDs which change with each pipeline reload. That means that with each pipeline reload, the PluginMetadata structure would retain stale data about old plugins and users could have difficulty identifying which entries refer to the currently-running plugins.

Also, not clearing PluginMetadata across pipeline reloads could result in the reporting of incorrect data if a plugin with a statically-defined ID in an updated pipeline does not correctly reset its own metadata. That seems like something that Logstash should make impossible by clearing PluginMetadata when a pipeline is reloaded.

#
# - It MUST NOT be used to store processing state
# - It SHOULD NOT be updated frequently.
# - Individual metadata keys MUST be Symbols and SHOULD NOT be dynamically generated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't know how important it is that the interface for PluginMetadata remain the same between Java and Ruby plugins, but Java plugins can't use symbols as keys.

@cachedout
Copy link
Copy Markdown
Contributor

Hi @yaauie and @danhermann . Any way that @elastic/stack-monitoring can help this along? Once we get it in, we can unblock the rest of our pending PRs for Logstash which in turn unblocks a fair amount of work on the Beats side. Thanks!

@jsvd
Copy link
Copy Markdown
Member

jsvd commented May 6, 2019

this work was moved to and finished in #10691

@jsvd jsvd closed this May 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants