8000 [collectd 6] lua: migrate the Lua plugin to v6.0 by kenhys · Pull Request #3795 · collectd/collectd · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[collectd 6] lua: migrate the Lua plugin to v6.0 #3795

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
Merged
merged 3 commits into from
Jan 13, 2024
Merged

[collectd 6] lua: migrate the Lua plugin to v6.0 #3795

merged 3 commits into from
Jan 13, 2024

Conversation

kenhys
Copy link
Contributor
@kenhys kenhys commented Dec 22, 2020 •
edited
Loading

ChangeLog: lua: migrate the Lua plugin to v6.0

Since v6, value_list_t is deprecated and metric_family_t is introduced.

metric_family_t is mapped to the following Lua table:

  {
    name => ...,
    help => ...,
    unit => ...,
    type => ...,
    resource => {
      key1 => value1,
      ...,
      keyN => valueN
    }
    metric => {
     [1] => {
       label => {
         key1 => value1,
         ...,
         keyN => valueN
       }
       value => ...,
       time => ...,
       interval => ...,
       meta => {
         key1 => value1,
         ...,
         keyN => valueN
       }
     },
     ...
     [N] => {
       ...
     }
  }

Closes: #3654

@collectd-bot collectd-bot added this to the 6.0 milestone Dec 22, 2020
@kenhys kenhys marked this pull request as ready for review December 27, 2020 07:40
@kenhys kenhys requested a review from a team as a code owner December 27, 2020 07:40
@kenhys kenhys force-pushed the v6-lua branch 2 times, most recently from 82ca95e to aada662 Compare December 30, 2023 12:59
@kenhys
Copy link
Contributor Author
kenhys commented Dec 30, 2023 •
edited
Loading

Rebased with recent collectd-6.0 branch:

Verified with:

sudo apt install -y libluajit-5.1-dev
git clone https://github.com/kenhys/collectd.git
cd collectd
git checkout -b v6-lua origin/v6-lua
export LIBLUA_PKG_CONFIG_NAME=luajit
./build.sh
./configure --prefix=/tmp/local --disable-all-plugins --enable-lua --enable-cpu
make install

Use conf and sample lua:

$ cat /legacy/collectd/test-v6.conf
PluginDir   "/tmp/local/lib/collectd"

LoadPlugin cpu

<LoadPlugin lua>
  Globals true
</LoadPlugin>

<Plugin lua>
  BasePath "/legacy/collectd/"
  Script "test-v6.lua"
</Plugin>

Use test-v6.lua something like:
(put inspect.lua under same directory)

$ cat /legacy/collectd/test-v6.lua
local inspect = require("inspect")
local PLUGIN_NAME = "Lua minimum plugin"

function read()
   collectd.log_info(PLUGIN_NAME .. ": read")
   return 0
end

# verified with only "LoadPlugins cpu"

function write(metrics)
   collectd.log_info(PLUGIN_NAME .. ": write")
   -- print(inspect(metrics))
   -- print(inspect(metrics["resource"]))
   assert(metrics["name"] == "system.cpu.time", "name must be system.cpu.time")
   assert(metrics["resource"], "resource must exist")
   assert(metrics["resource"]["host.id"], "host.id must exist")
   assert(metrics["resource"]["host.name"], "host.name must exist")
   assert(metrics["resource"]["service.name"], "service.name must exist")
   assert(metrics["type"] == 0, "type must be 0")
   assert(metrics["unit"] == "s", "type must be 's'")
   -- print(#metrics["metric"])
   assert(#metrics["metric"] > 0, "metric must be contained")
   for index, metric in pairs(metrics["metric"]) do
      assert(metric["interval"] == 10, "interval must be 10")
      assert(metric["time"] > 0, "time must be greater than 0")
      assert(metric["meta"], "meta must exist")
      -- print(inspect(metrics["meta"]))
      assert(metric["value"] >= 0, "value must be greater than 0")
      assert(metric["label"]["cpu"], "label must contain cpu")
      assert(metric["label"]["state"], "label must contain state")
   end
   return 0
end

collectd.register_read(read)
collectd.register_write(write)

/tmp/local/sbin/collectd -f -C test-v6.conf should work without error.

@kenhys
Copy link
Contributor Author

@octo could you afford to review this PR?

@octo octo self-requested a review December 30, 2023 21:07
octo
octo reviewed Dec 30, 2023
View reviewed changes
Copy link
Member
@octo octo left a comment

Choose a reason for hiding this comment

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

Overall this looks great, thanks @kenhys!

My biggest concern is that label sets and meta data is mapped to a list of one-element maps, rather than just a map; see discussion inline.

Best regards and a happy new year 🎆
—octo

@kenhys kenhys force-pushed the v6-lua branch 2 times, most recently from 6556fdc to 2127b7c Compare December 31, 2023 08:49
octo
octo reviewed Jan 1, 2024
View reviewed changes
@kenhys kenhys force-pushed the v6-lua branch 2 times, most recently from 13da451 to ebb8cad Compare January 1, 2024 11:57
@eero-t
Copy link
Contributor
eero-t commented Jan 2, 2024

Some test code for this functionality would also be nice, but that could be a separate PR.

@octo
Copy link
Member
octo commented Jan 3, 2024

+1, tests would be nice to have, but I won't block on them.

I do think that we need to update the documentation a bit though. Could you document the updated behavior in src/collectd-lua.pod? I think it would be ideal to add a "Data Types" section and there talk about how metric_family_t is mapped to Lua. I think with that change this should be good to go.

Thanks for your continued work and best regards
—octo

kenhys added 2 commits January 13, 2024 14:39
@kenhys
lua: enable Lua
e6be1e4 
Signed-off-by: Kentaro Hayashi <kenhys@gmail.com>
@kenhys
lua: migrate interface to v6 plugin API
e116230 
Since v6, value_list_t is deprecated and metric_family_t is
introduced.

metric_family_t is mapped to the following Lua table:

  {
    name => ...,
    help => ...,
    unit => ...,
    type => ...,
    resource => {
      key1 => value1,
      ...,
      keyN => valueN
    }
    metric => {
     [1] => {
       label => {
         key1 => value1,
         ...,
         keyN => valueN
       }
       value => ...,
       time => ...,
       interval => ...,
       meta => {
         key1 => value1,
         ...,
         keyN => valueN
       }
     },
     ...
     [N] => {
       ...
     }
  }

Closes: collectd#3654

Signed-off-by: Kentaro Hayashi <kenhys@gmail.com>
@kenhys
lua: update data types section
67c3aa5 
It describes changes about internal data type.

Signed-off-by: Kentaro Hayashi <kenhys@gmail.com>
@kenhys
Copy link
Contributor Author
kenhys commented Jan 13, 2024

I've added explanation about data types and rebased with collectd-6.0.

@kenhys
Copy link
Contributor Author
kenhys commented Jan 13, 2024

Except "Labels", CI has passed.

@kenhys kenhys requested a review from octo January 13, 2024 08:11
@octo octo added the Feature label Jan 13, 2024
octo 8000
octo approved these changes Jan 13, 2024
View reviewed changes
Copy link
Member
@octo octo left a comment

Choose a reason for hiding this comment

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

Looks greak, thanks @kenhys!

@octo octo added the Automerge Labels PRs to be merged by a bot once approved label Jan 13, 2024
@octo octo merged commit 29935eb into collectd:collectd-6.0 Jan 13, 2024
@kenhys kenhys deleted the v6-lua branch January 13, 2024 10:07
@kenhys kenhys mentioned this pull request Jan 13, 2024
Open
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@octo octo octo approved these changes

Assignees
No one assigned
Labels
Automerge Labels PRs to be merged by a bot once approved Feature
Projects
collectd 6
Status: Done
Milestone
6.0
Development

Successfully merging this pull request may close these issues.
4 participants
@kenhys @eero-t @octo @collectd-bot
0