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

postgresql: Enable to treat meta data at writer #3227

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

postgresql: Enable to treat meta data at writer #3227

wants to merge 4 commits into from
+477 −6

Conversation

ashie
Copy link
Contributor

@ashie ashie commented Jul 22, 2019 •
edited

ChangeLog: postgresql: Enable to handle meta data at writer

Although users may want to handle some meta data at SQL world but postgresql plugin doesn't provide a way to do it. For example a user may want to write network:username or network:ip_address to DB.

This patch add a new optional argument $10 on executing a SQL command to pass a JSON string which stores meta data. $10 is added only when MetaData option is added to <Writer> so that users of old versions can safely upgrade to this version.

@dago
Copy link
Contributor

dago commented Feb 25, 2020

Other plugins already use libjansson for JSON writing, maybe for the sake of consistency it would be good to use that here too?

@ashie
Copy link
Contributor Author

ashie commented Feb 26, 2020 •
edited

Other plugins already use libjansson for JSON writing, maybe for the sake of consistency it would be good to use that here too?

Thanks for your comment!
It's reasobale, I'll address it.

@ashie
Copy link
Contributor Author

ashie commented Feb 28, 2020 •
edited

Hmm, libjansson is optional and only dpdk_telemetry plugin uses it.
On the other hand there is a built-in API to format JSON (src/utils/format_json), and some plugins uses it.
Although former one has better API, I don't want to make this feature optional. So I'll choose later one (add a wrapper function to expose meta_data_to_json() to public then use it).

@ashie
Copy link
Contributor Author

ashie commented Feb 28, 2020

In addition, I'll update the document & sample schema to clarify that $10 is optional.

@ashie ashie force-pushed the postgresql-write-metadata branch from 6731334 to 74ab8ee Compare July 2, 2020 04:05
@ashie ashie requested a review from a team as a code owner July 2, 2020 04:05
@ashie
Copy link
Contributor Author

ashie commented Jul 2, 2020

So I'll choose later one (add a wrapper function to expose meta_data_to_json() to public then use it).

I added a new function format_json_meta_data() for it, but it's not enough for my usage yet.
Because I need to select json keys to dump. I'll add this feature to this function.

@ashie ashie force-pushed the postgresql-write-metadata branch from 74ab8ee to 6bcf750 Compare July 2, 2020 04:24
@ashie
Copy link
Contributor Author

ashie commented Jul 2, 2020 •
edited

TODO list:

  • Add selected_keys argument to format_json_meta_data()
  • Replace metadata_to_json() with format_json_meta_data()
  • Update the document to clarify that $10 is optional.
  • Update the sample schema to clarify that $10 is optional.
  • Add a new sample schema to store metadata to DB
  • Test the modified version

@collectd-bot collectd-bot changed the base branch from master to main July 3, 2020 09:40
@ashie ashie force-pushed the postgresql-write-metadata branch 16 times, most recently from 841e7c8 to d76b2cb Compare August 26, 2020 01:17
@ashie
Copy link
Contributor Author

ashie commented Aug 26, 2020

@collectd/trusted-contributors @dago or someone.
I've completed to rewrite this pull request.
Could you please review again if there is still a chance to merge it?

@kenhys kenhys added the Feature label Jan 27, 2024
@ashie @kenhys
Add a new function to format meta_data_t as JSON
26099d7 
Signed-off-by: Takuro Ashie <ashie@clear-code.com>
ashie added 3 commits May 4, 2024 11:55
@ashie @kenhys
postgresql plugin: Enable to store meta data by Writer
268ab63 
You need to add the following config into you Writer section to enable
this feature:

  MetaData true

This option will store all meta data in a metric.
Or you can specify each meta data like the following setting:

  MetaData "network:username" "network:ip_address" ...

Signed-off-by: Takuro Ashie <ashie@clear-code.com>

postgresql: Use format_json_meta_data() to format meta data

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
@ashie @kenhys
postgresql plugin: Update document
e0d2d25 
Add description about "MetaData" of "Writer".

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
@ashie @kenhys
postgresql: Add a sample schema file to utilize metadata
5e95fba 
It requires PostgreSQL 9.4 or later since it uses JSONB data type.

Signed-off-by: Takuro Ashie <ashie@clear-code.com>
@kenhys kenhys force-pushed the postgresql-write-metadata branch from 17b2afd to 5e95fba Compare May 4, 2024 02:56
@kenhys
Copy link
Contributor

kenhys commented May 4, 2024

NOTE: CI build/experimental fails because of the following issue, it has nothing to do with this PR change.

fix(epics): Fix include path -- at least on Debian unstable.
#4298

@kenhys
Copy link
Contributor

kenhys commented May 4, 2024

NOTE: CI build/experimental fails because of the following issue, it has nothing to do with this PR change.

I've predicate that It takes a longer time to be fixed (Build experimental issue See #4298)

Could you review @octo ? (except above issue)

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
Feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ashie @dago @kenhys