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 possibility to configure log format #799 #2941

Merged
merged 66 commits into from
Dec 23, 2024

Conversation

victoredvardsson
Copy link
Contributor

@victoredvardsson victoredvardsson commented Apr 14, 2024

#799

logrus which is used as logging backend supports json formatter.

This change makes it possible to configure either text or json output.

Copy link

@victoredvardsson: There are no 'kind' label on this PR. You need a 'kind' label to generate the release automatically.

  • /kind feature
  • /kind enhancement
  • /kind fix
  • /kind chore
  • /kind dependencies
Details

I am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository.

Copy link

@victoredvardsson: There are no area labels on this PR. You can add as many areas as you see fit.

  • /area agent
  • /area local-api
  • /area cscli
  • /area appsec
  • /area security
  • /area configuration
Details

I am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository.

@victoredvardsson
Copy link
Contributor Author

/kind feature

@victoredvardsson
Copy link
Contributor Author

/area configuration

@victoredvardsson victoredvardsson marked this pull request as draft April 15, 2024 07:02
@victoredvardsson victoredvardsson marked this pull request as ready for review April 15, 2024 08:10
@buixor buixor modified the milestones: 1.6.4, 1.6.5 Nov 15, 2024
@luis-guimaraes-exoawk
Copy link

Any news on this feature? It's very important for us

@buixor buixor added the value/high Doing this significantly improves some areas label Dec 18, 2024
@LaurenceJJones
Copy link
Contributor

Any news on this feature? It's very important for us

aiming for 1.6.5 which is the next release

@mmetc mmetc requested a review from LaurenceJJones December 20, 2024 14:07
@LaurenceJJones
Copy link
Contributor

Tested and works ✅

The only comments is that some log lines contain special characters or simply alot of information that we should use parameter instead of containing it all within the msg objects examples:

{"level":"debug","msg":"Response: HTTP/2.0 200 OK\r\nContent-Length: 876\r\nContent-Type: application/json\r\nDate: Sun, 22 Dec 2024 11:38:52 GMT\r\nVia: 1.1 e4faf88ed08954d3c5034fc535379dc6.cloudfront.net (CloudFront), 1.1 936c7ee6d0620cb8a766a50c04b3fa30.cloudfront.net (CloudFront)\r\nX-Amz-Apigw-Id: DMQL1EXWjoEEAHQ=\r\nX-Amz-Cf-Id: u0EUeH2MMqcUnhnI21rrAGYrOYyZnlWiZtBb9gPGVFRH_LD1qCGnew==\r\nX-Amz-Cf-Pop: LHR5-P7\r\nX-Amz-Cf-Pop: LHR50-P3\r\nX-Amzn-Requestid: 10f480e2-6468-404a-b365-5cc7ac8253a0\r\nX-Amzn-Trace-Id: Root=1-6767fa4b-0230272b064ebb435b2cd65e;Parent=1eaae8e82d2f732c;Sampled=0;Lineage=1:80359e24:0\r\nX-Cache: Miss from cloudfront\r\n\r\n{\"code\": 200, \"token\": \"<redacted>\", \"expire\": \"2024-12-22T19:38:52Z\"}","time":"2024-12-22T11:38:52Z"}

We should add a type parameter type=request or type=response and the response should be message 🤷🏻

Also some of the special characters get turned into the html safe equivalent (\u003e\u003e):

{"level":"debug","msg":"starting one source 0/2 -\u003e\u003e *fileacquisition.FileSource","time":"2024-12-22T11:38:52Z"}
{"level":"debug","msg":"starting one source 1/2 -\u003e\u003e *fileacquisition.FileSource","time":"2024-12-22T11:38:52Z"}

We should just not use special characters in message objects as they add no value but this can be done in another PR as these are nothing to do with the actual PR just a side effect of legacy things.

@mmetc
Copy link
Contributor

mmetc commented Dec 23, 2024

agree on both points

  • unnecessary use of < and >
  • more structured logging

can we merge this and work on the above for 1.6.6 / 1.7.0 ?

Copy link
Contributor

@LaurenceJJones LaurenceJJones left a comment

Choose a reason for hiding this comment

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

LGTM and tested 🦾

@mmetc mmetc merged commit 466f39b into crowdsecurity:master Dec 23, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/configuration kind/feature question Further information is requested value/high Doing this significantly improves some areas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants