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.

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

feat(outputs): add hostname, tags, custom, and templated fields to TimescaleDB #438

Merged
merged 1 commit into from
Apr 26, 2023

Conversation

alika
Copy link
Contributor

@alika alika commented Mar 28, 2023

What type of PR is this?
/kind feature

Any specific area of the project related to this PR?
/area outputs

What this PR does / why we need it:
This PR adds support for hostname, tags, custom fields, and templated fields for the TimescaleDB output.

Which issue(s) this PR fixes:

Fixes #

README.md Show resolved Hide resolved
@Issif Issif added this to the 2.28.0 milestone Mar 28, 2023
Copy link
Member

@Issif Issif left a comment

Choose a reason for hiding this comment

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

I see the output fields are not added, because of the the schema. Do you think it worth to add k8s.pod.name and k8s.ns.name at least? IN prometheus for example, to avoid huge cardinality, we add these fields as labels and the custom fields.

@alika
Copy link
Contributor Author

alika commented Mar 28, 2023

I see the output fields are not added, because of the the schema. Do you think it worth to add k8s.pod.name and k8s.ns.name at least? IN prometheus for example, to avoid huge cardinality, we add these fields as labels and the custom fields.

Sure, sounds reasonable. I'll add support for output fields.

@Issif
Copy link
Member

Issif commented Mar 30, 2023

I see the output fields are not added, because of the the schema. Do you think it worth to add k8s.pod.name and k8s.ns.name at least? IN prometheus for example, to avoid huge cardinality, we add these fields as labels and the custom fields.

Sure, sounds reasonable. I'll add support for output fields.

All output fields it will be hard, because they are dynamic, but at least the custom fields added statically by Falcosidekick BUT it means you need to deal with the addition of new fields after the creation of the index, ie, a schema update.

Tell me when it's ready, I'll review this PR at this moment. Thanks.

@alika
Copy link
Contributor Author

alika commented Mar 30, 2023

I see the output fields are not added, because of the the schema. Do you think it worth to add k8s.pod.name and k8s.ns.name at least? IN prometheus for example, to avoid huge cardinality, we add these fields as labels and the custom fields.

Sure, sounds reasonable. I'll add support for output fields.

All output fields it will be hard, because they are dynamic, but at least the custom fields added statically by Falcosidekick BUT it means you need to deal with the addition of new fields after the creation of the index, ie, a schema update.

Tell me when it's ready, I'll review this PR at this moment. Thanks.

After I looked at how newFalcoPayload works I understand how OutputFields is already populated w/Customfields and Templatedfields. I have an implementation right now that only adds to TimescaleDB payload if it is declared in either Customefields or Templatedfields. Just doing a bit more actual testing and I'll have this PR updated soon (<24h).

…plated fields to TimesacleDB output

Signed-off-by: alika <alika.larsen@gmail.com>
@alika alika force-pushed the feat-timescaledb-customfields branch from ec4241e to 3d3e644 Compare March 30, 2023 20:00
@alika
Copy link
Contributor Author

alika commented Mar 30, 2023

@Issif ready for review now. I had to touch config because I was getting a panic on templated fields:

panic: assignment to entry in nil map

goroutine 1 [running]:
main.getConfig()
        /Users/alika/projects/ref/falcosidekick/config.go:474 +0x3eeb
main.init.0()
        /Users/alika/projects/ref/falcosidekick/main.go:89 +0xff

@alika alika requested a review from Issif March 30, 2023 20:04
@alika alika changed the title feat(outputs): add hostname, tags, and custom fields to TimesacleDB feat(outputs): add hostname, tags, custom, and templated fields to TimesacleDB Mar 30, 2023
@Issif Issif changed the title feat(outputs): add hostname, tags, custom, and templated fields to TimesacleDB feat(outputs): add hostname, tags, custom, and templated fields to TimescaleDB Mar 31, 2023
@Issif
Copy link
Member

Issif commented Apr 24, 2023

Sorry for the delay, I was successively sick, off and at kubecon in last weeks. Have you fixed the issue with the nil map? Can I review the whole PR now? Thanks

@alika
Copy link
Contributor Author

alika commented Apr 26, 2023

Sorry for the delay, I was successively sick, off and at kubecon in last weeks. Have you fixed the issue with the nil map? Can I review the whole PR now? Thanks

No worries. Hope kubecon was fun. I did fix this issue w/the nil map and I'm ready for a review.

@poiana
Copy link

poiana commented Apr 26, 2023

LGTM label has been added.

Git tree hash: a3aac9de46e6817dc30db3c9e2b7d2150577bb20

@poiana
Copy link

poiana commented Apr 26, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alika, Issif

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana merged commit 215930f into falcosecurity:master Apr 26, 2023
@alika alika deleted the feat-timescaledb-customfields branch April 26, 2023 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants