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: Json layout modern implementation #670

Merged
merged 4 commits into from
Dec 27, 2021

Conversation

pankajagrawal16
Copy link
Contributor

@pankajagrawal16 pankajagrawal16 commented Dec 25, 2021

Issue #, if available:

Description of changes:

Json Template Layout is recommended way of doing and extending structured logging in log4j now.

JsonTemplateLayout is a customizable, efficient, and garbage-free JSON generating layout. It encodes LogEvents according to the structure described by the JSON template provided. In a nutshell, it shines with its

  • Customizable JSON structure (see eventTemplate[Uri] and stackTraceElementTemplate[Uri] layout configuration parameters)

  • Customizable timestamp formatting (see timestamp event template resolver)

  • Feature rich exception formatting (see ] and xref:event-template-resolver-exceptionRootCause[ event template resolvers)

  • Extensible plugin support

  • Customizable object recycling strategy

Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@pankajagrawal16
Copy link
Contributor Author

@humanzz Have a look


```properties hl_lines="1 2"
log4j.layout.jsonTemplate.timestampFormatPattern=yyyy-MM-dd'T'HH:mm:ss.SSSZz
log4j.layout.jsonTemplate.timeZone=Europe/Oslo
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to actually have that default to UTC unless overridden?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rite now the behaviour is consistent with existing implementation, meaning it will pick system default timezone. But with new approach, users have ability to customise timezone if need be, which was not possible before.

@heitorlessa
Copy link
Contributor

heitorlessa commented Dec 26, 2021 via email

@pankajagrawal16
Copy link
Contributor Author

pankajagrawal16 commented Dec 26, 2021

We could have an env var, otherwise it’d be a breaking change (same
happened in Python).

For v2, we could consider it having as default, tho I was surprised by how
many still stick with local time (default in Lambda

It won't be a breaking change, because existing functionality og using deprecated implementation will work as is without any change.

With upgrade to new layout configuration, behaviour will still remain the same.

@heitorlessa
Copy link
Contributor

heitorlessa commented Dec 26, 2021 via email

@pankajagrawal16
Copy link
Contributor Author

Oh no Pankaj, what I meant about breaking change is in response to the
question of - “why not making UTC the default?”

For Python, customers can set utc=True as an opt-in, else we use local tz

Oh yeah. Then i misunderstood. Definitely that is an option but then again with new approach users can do more natively using log4j config. But yeah we can spport that via environment variables config as well if need be.

@heitorlessa
Copy link
Contributor

heitorlessa commented Dec 26, 2021 via email

@pankajagrawal16 pankajagrawal16 force-pushed the json-layout-modern-implementation branch from 66f9695 to 3063711 Compare December 27, 2021 09:19
@pankajagrawal16 pankajagrawal16 merged commit 8f18865 into master Dec 27, 2021
@pankajagrawal16 pankajagrawal16 deleted the json-layout-modern-implementation branch December 27, 2021 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants