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.

Sign up for GitHub

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

Jump to bottom

Create new plugin API abstractions #1618

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft

Create new plugin API abstractions #1618

wants to merge 18 commits into from

Conversation

morrone
Copy link
Collaborator

@morrone morrone commented Feb 21, 2025 

Develop better plugin API abstractions.

Make read-only ldmsd's access to the API function pointer structs: struct ldmsd_plugin,
struct ldmsd_store, and struct ldmsd_sampler. The plugins now own those structs.

Introduce the ldmsd_plug_api.[ch] files, which most importantly include the accessor
functions for the new ldmsd_plug_handle_t, which is a opaque to the plugins.

ldmsd passes an opaque ldmsd_plug_handle_t to the plugins in each plugin API call.

Plugins may use the accessor functions to get and set things through the handle.

In particular, a plugin may use ldmsd_plug_context_set() to record its local context
pointer in one call, and use ldmsd_plug_context_get() to retrieve its local context
pointer.

Functions calls constructor() and destructor() are added to the plugin API to simplify
plugin multi-instance support.

The filesingle sampler is upgraded to multi-instance support.

tom95858 and others added 17 commits January 24, 2025 13:19
@tom95858 @narategithub
Multi-instance plugin configuration support
de001f1 
* Converted plugins to configuration objects
* Added multi-instance support to samplers:
  * procnetdev2
  * meminfo
  * avro_kafka store
  * csv store
  * sos store
* Converted all samplers to support new plugin infrastructure
* Changed configuration object reference counting to use ref_t
Fix strgp_open() always been called even for decomposition
3ea7bbd 
The `strgp_open()` is only for legacy store path. When the plugin is
configured with `decomp_path`, this legacy strgp path should not be
executed.
Fix store_sos: wrong pointer & uninitialized si->store
ba0f5fd
Fix store_avro_kafka
76d6530 
- default 'encoding' to AVRO
- sk->g_topic_fmt not set if FMT not given
Fix store_csv
833d9b5
Fix plugin term
bb8a017
Fix procnetdev2
897a300
Fix memory leak in ldmsd_set_deregister()
dc3f1ea
Add cfgobj pointer to ldmsd_plugin instance
8fd7c4b 
Add `cfgobj` pointer to `ldmsd_plugin` instance so that the plugin can
take `cfgobj` reference. This is required by `json_stream_sampler` to
prevent use-after-free in the race of destroying the plugin instance and
delivering events on the stream client that still hold the reference to
the plugin instance.
Fix json_stream_sampler
c2d0e8a
Fix cfgobj lock held in plugin alloc
1e26bb1
@tom95858
Modify the syntax of the load command
15dd64a 
This change modifies the load command as follows:

  load name=config_object_name plugin=plugin_name

If 'plugin' is not specified, it defaults to the 'config_object_name'.

For example:
    load name=mem1 plugin=meminfo
    load name=meminfo
    plugn_status

Results in the followihg:

Config Name  Plugin                   Type         libpath
------------ ------------------------ ------------ ------------
mem1         meminfo                  sampler      /opt/ovis/lib64/ovis-ldms/libmeminfo.so
meminfo      meminfo                  sampler      /opt/ovis/lib64/ovis-ldms/libmeminfo.so

From: Chris Morrone, [email protected]
@tom95858
Rename inst_name to cfg_name
0b4822c 
The inst_name in the configuration object is confusing considering
that the inst_name in other contexts refers to a set. This change
uses cfg_name that makes it obvious that the name refers to a
configuration object.

From: Chris Morrone, [email protected]
@tom95858
Remove cfgobj pointer from ldmsd_plugin structure
362972e 
From: Chris Morrone, [email protected]
@tom95858
Remove redundant plugin_name parameter checking
3955df4 
From: Chris Morrone, [email protected]
@tom95858
Fix the set_route command
59262f0
@tom95858
Rename "inst" structs to "cfgobj"
2c353ed 
* Rename "struct ldmsd_sampler_inst" to "struct ldmsd_cfgobj_sampler".
* Rename ldmsd_sampler_t to ldmsd_cfgobj_sampler_t (to match previous).
* Rename "struct ldmsd_store_inst" to "struct ldmsd_cfgobj_store".
* Rename ldmsd_store_t ldmsd_cfgobj_store_t (to match previous).

These changes better reflect that these struct are derived from
the base struct ldmsd_cfgobj.

The typedef names now directly match the underlying struct, with
just "_t" appended to the name. Previously the typedef names did
not match the struct names.

From: Chris Morrone, [email protected]

Remove the set_route command.

From: Tom Tucker, [email protected]
@morrone morrone mentioned this pull request Feb 21, 2025
Closed
@morrone morrone force-pushed the new_plugin_api branch 8 times, most recently from 260160d to 4595f82 Compare February 21, 2025 00:53
@morrone
Copy link
Collaborator Author

morrone commented Feb 21, 2025

This builds, but has virtually no testing yet. I share the draft now just so folks know where I'm heading and can provide early feedback.

@morrone
Create new plugin API abstractions
a83da10 
Develop better plugin API abstractions.

Make read-only ldmsd's access to the API function pointer structs: struct ldmsd_plugin,
struct ldmsd_store, and struct ldmsd_sampler. The plugins now own those structs.

Introduce the ldmsd_plug_api.[ch] files, which most importantly include the accessor
functions for the new ldmsd_plug_handle_t, which is a opaque to the plugins.

ldmsd passes an opaque ldmsd_plug_handle_t to the plugins in each plugin API call.

Plugins may use the accessor functions to get and set things through the handle.

In particular, a plugin may use ldmsd_plug_context_set() to record its local context
pointer in one call, and use ldmsd_plug_context_get() to retrieve its local context
pointer.

Functions calls constructor() and destructor() are added to the plugin API to simplify
plugin multi-instance support.

Many plugin functions are now optional. ldmsd checks if they are NULL before calling
them. Therefore plugins no longer need to implement empty stub functions that do
nothing.

The filesingle sampler is upgraded to multi-instance support.
@morrone morrone added this to the v4.5.1 milestone Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v4.5.1
Development

Successfully merging this pull request may close these issues.
3 participants
@morrone @tom95858 @narategithub