Skip to content

LOG4J2-3116 Google Cloud structured logging via JsonTemplate #543

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

Merged
merged 7 commits into from
Jul 20, 2021

Conversation

rocketraman
Copy link
Member

No description provided.

@rocketraman rocketraman changed the title Google Cloud structured logging via JsonTemplate LOG4J2-3116 Google Cloud structured logging via JsonTemplate Jul 1, 2021
@rocketraman rocketraman requested a review from vy July 7, 2021 14:41
@vy
Copy link
Member

vy commented Jul 19, 2021

@rocketraman, I have pushed my changes to #551. I had already started working on the ticket before you had submitted your PR, hence I wanted to stick to that. Nevertheless, credit still goes to you in changes.xml. #551 has the following differences:

  • exception_class, exception_message, and exception_stacktrace are respectively renamed to class, message, and stackTrace.
  • Uses CounterResolver for logging.googleapis.com/insertId.
  • Contains more versatile tests.

I am happy with the final state of #551. If you approve, please go ahead and squash+rebase it onto release-2.x. I would also appreciate it if you can cherry-pick it onto master, otherwise I can deal with it myself too.

@rocketraman
Copy link
Member Author

@rocketraman, I have pushed my changes to #551. I had already started working on the ticket before you had submitted your PR, hence I wanted to stick to that. #551 has the following differences:

I'm a little confused as to why you would have started work on the PR when you specifically asked me to do so on the dev mailing list when I proposed this modification, and I did that less than 3 hours later: https://lists.apache.org/thread.html/rdc25435870e47284564575b53d3b0dbf69e33ae7736f388878b89bf0%40%3Cdev.logging.apache.org%3E.

Nevertheless, credit still goes to you in changes.xml

I do appreciate that, though it would have been nice to get the "commit credit" as well. Nevertheless, not a big deal.

#551 has the following differences:

I'll take a look shortly.

@vy
Copy link
Member

vy commented Jul 19, 2021

@rocketraman, sorry for the mess. I very well see your point for commit credit. I will base #551 on top of #543.

@vy vy reopened this Jul 20, 2021
@vy vy merged commit a950ba8 into apache:release-2.x Jul 20, 2021
@rocketraman
Copy link
Member Author

@rocketraman, sorry for the mess. I very well see your point for commit credit. I will base #551 on top of #543.

I appreciate that @vy. I see you've merged this in already, awesome!

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.

2 participants