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

Add log to metrics plugin #1305

Merged
merged 2 commits into from
Aug 20, 2024

Conversation

Athishpranav2003
Copy link
Contributor

What this PR does / why we need it:

Add LogToMetrics Filter plugin

Which issue(s) this PR fixes:

Fixes #1285

Does this PR introduced a user-facing change?

Nope

Add `LogToMetrics` Filter plugin to Fluentbit component 

Additional documentation, usage docs, etc.:

Updated in the codebase itself.

@Athishpranav2003 Athishpranav2003 force-pushed the add-log-to-metrics-plugin branch from a122fc4 to 98b000a Compare August 15, 2024 04:02
@Athishpranav2003
Copy link
Contributor Author

@cosmo0920 the build is failing for some other reason. I have raised PR so that we can get started with the discussions. Meanwhile i will try to look at the build issue. Also running make is actually modifying some other files dont know why. Would need your help in this part.

Do we need to add tests for this plugin?

@Athishpranav2003 Athishpranav2003 force-pushed the add-log-to-metrics-plugin branch from 98b000a to c486362 Compare August 16, 2024 02:21
@Athishpranav2003 Athishpranav2003 force-pushed the add-log-to-metrics-plugin branch 2 times, most recently from fc28e76 to 3d64acc Compare August 16, 2024 02:32
@Athishpranav2003
Copy link
Contributor Author

@benjaminhuo Now the local build is working. I guess make is autoformatting the code and wantedly changing

        modified:   apis/fluentbit/v1alpha2/plugins/filter/zz_generated.deepcopy.go
	modified:   apis/fluentbit/v1alpha2/zz_generated.deepcopy.go
	modified:   go.mod
	modified:   go.sum

Not sure why. I removed those changes and then pushed it for now

@Athishpranav2003
Copy link
Contributor Author

@benjaminhuo push to ghcr seems to fail (403 error code). I guess thats issue with some configuration in gcp i guess. Does this seem fine now?

@Athishpranav2003
Copy link
Contributor Author

@cosmo0920 Could you check the PR? i have addressed your comments

wenchajun
wenchajun previously approved these changes Aug 19, 2024
cosmo0920
cosmo0920 previously approved these changes Aug 19, 2024
@benjaminhuo
Copy link
Member

Not sure why. I removed those changes and then pushed it for now

The auto-generated files shouldn't be removed.
You should run make manifests generate fmt vet docs-update or even make test docs-update and commit everything generated @Athishpranav2003

@Athishpranav2003
Copy link
Contributor Author

Ok fine
Will do it and update

@Athishpranav2003
Copy link
Contributor Author

@benjaminhuo Have added them too as well

cosmo0920
cosmo0920 previously approved these changes Aug 19, 2024
@cosmo0920
Copy link

I put an approve and kicked CI tasks as well.

@Athishpranav2003
Copy link
Contributor Author

@cosmo0920 seems like its passing the tests. Anything else i missed?

wenchajun
wenchajun previously approved these changes Aug 19, 2024
@Athishpranav2003 Athishpranav2003 dismissed stale reviews from wenchajun and cosmo0920 via 38a2605 August 19, 2024 10:59
@Athishpranav2003 Athishpranav2003 force-pushed the add-log-to-metrics-plugin branch from 0bc2819 to 38a2605 Compare August 19, 2024 10:59
@Athishpranav2003
Copy link
Contributor Author

@benjaminhuo addressed your comments. I wasnt sure about the convention part that you guys followed

@benjaminhuo
Copy link
Member

@benjaminhuo addressed your comments. I wasnt sure about the convention part that you guys followed

Thanks for the contribution! @Athishpranav2003

@benjaminhuo benjaminhuo merged commit a327d9b into fluent:master Aug 20, 2024
9 of 10 checks passed
@Athishpranav2003 Athishpranav2003 deleted the add-log-to-metrics-plugin branch August 20, 2024 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fluent-bit: Add log_to_metrics filter plugin
4 participants