Skip to content

Coralogix processor first PR #33458

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 50 commits into from
Aug 9, 2024
Merged

Coralogix processor first PR #33458

merged 50 commits into from
Aug 9, 2024

Conversation

galrose
Copy link
Contributor

@galrose galrose commented Jun 10, 2024

Description:
Adding a feature - Adding a feature to create templates (blueprints) from sql queries.
Currently specifically for postgresql and mysql queries.

Link to tracking Issue: #33090

Testing:
currently no tests, will be added in next PR

Documentation:
Added documentation for possible configuration and the usecase of the processor

@galrose galrose requested review from a team and mx-psi June 10, 2024 15:02
@linux-foundation-easycla Linux Foundation: EasyCLA
Copy link

linux-foundation-easycla bot commented Jun 10, 2024 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@crobert-1
Copy link
Member

This PR should have a changelog entry. 👍

crobert-1
crobert-1 reviewed Jun 10, 2024
View reviewed changes
Copy link
Member

@crobert-1 crobert-1 left a comment

Choose a reason for hiding this comment

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

Looks good overall. I've left a lot of comments on the README, the goal is to be able to flesh out UI/UX up front, before getting too far into the implementation weeds.

There are also a number of CI/CD failures caused by this PR, so please take a look and resolve when you're able. Let me know if you run into any issues there, happy to help!

@mx-psi mx-psi removed their request for review June 13, 2024 10:36
@galrose
Copy link
Contributor Author

galrose commented Jun 13, 2024

Hey @crobert-1, thanks for the review!
I did some changes as you requested and I would like to rerun the CI to take a look if some of the issues were fixed.
Im also not sure if I removed enough from span.go or if I should have removed more code.

Thanks for the help 😄

@galrose galrose requested a review from crobert-1 June 13, 2024 12:55
crobert-1
crobert-1 reviewed Jun 13, 2024
View reviewed changes
@galrose galrose requested a review from crobert-1 June 17, 2024 07:44
crobert-1
crobert-1 reviewed Jun 17, 2024
View reviewed changes
@galrose
Copy link
Contributor Author

galrose commented Jun 18, 2024

@crobert-1 can you please help me understand why I don't pass the CI?
I am running all the scripts from the first PR section and they all look fine locally

@crobert-1
Copy link
Member

@crobert-1 can you please help me understand why I don't pass the CI? I am running all the scripts from the first PR section and they all look fine locally

You'll have to look through the different failed actions individually to see why they're failing. From what I can tell they're related to this PR.

One example:

license header checking failed:
./config.go
./config.go
./config.go
./config.go
./factory.go
./factory.go
./factory.go
./factory.go
./span.go
./span.go
./span.go
./span.go
make[2]: *** [../../Makefile.Common:174: checklicense] Error 1
make[1]: *** [Makefile:179: processor/coralogixprocessor] Error 2

This means these files don't have the proper license header. If you look in Makefile.Common you can see what this check is doing. You need to run make addlicense to resolve this.

For this failure:

FAIL: "processor/coralogixprocessor" not included in CODEOWNERS

The command make update-codeowners needs to be run, as described in the CONTRIBUTING doc.

There are more failures that need investigated, but let me know if there's anything you don't understand 👍

@galrose galrose requested a review from crobert-1 July 10, 2024 07:49
@galrose
Copy link
Contributor Author

galrose commented Jul 11, 2024

@atoulme I see I need your approval as well for the code owners 😄