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

Update MDC handling in the AmqpAppender #1014

Closed
egusev opened this issue Jun 6, 2019 · 3 comments
Milestone

Comments

@egusev
Copy link
Contributor

@egusev egusev commented Jun 6, 2019

For proper logging additional information of calling paramenters/context is required. The best option to do that is MDC.
The current implementation copies MDC into the message's headers.
The main problem with it that Rabbit has limited size for header and default size is pretty small - 131072 bytes (128 kb). unlike of message body it can't be fragmented spring-cloud/spring-cloud-stream-binder-rabbit#159

so the proposals are:

  1. remove (or make it optional) serializating MDC into headers
  2. add possibility to add MDC into the message body
  3. make AmqpAppender.sendEvent protected (instead of private) to allow override it to do custom stuff
@egusev

This comment has been minimized.

Copy link
Contributor Author

@egusev egusev commented Jun 6, 2019

I mean this class org.springframework.amqp.rabbit.log4j2.AmqpAppender

@egusev

This comment has been minimized.

Copy link
Contributor Author

@egusev egusev commented Jun 6, 2019

update: JsonLayout already serializes MDC into the body. so only points 1 and 3 are actual

@garyrussell garyrussell added this to the 2.2.x milestone Jun 6, 2019
@garyrussell

This comment has been minimized.

Copy link
Member

@garyrussell garyrussell commented Jun 6, 2019

egusev added a commit to egusev/spring-amqp that referenced this issue Jun 7, 2019
…g4j2 appender to turn on the serialization of MDC into header. by default it turned off

spring-projects#1014
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 7, 2019
…g4j2 appender to turn on the serialization of MDC into header. by default it turned off

spring-projects#1014
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 7, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 7, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 7, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 7, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 7, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 7, 2019
…g4j2 appender to turn on the serialization of MDC into header. by default it turned off

spring-projects#1014
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 7, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 7, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 7, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 8, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 8, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 8, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 8, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 8, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 8, 2019
egusev added a commit to egusev/spring-amqp that referenced this issue Jun 13, 2019
artembilan added a commit that referenced this issue Jun 13, 2019
Fixes #1014

**Cherry-pick to 2.1.x, 2.0.x & 1.7.x**

GH-1014 minor changes

GH-1014 renamed property to addMdcAsHeaders

GH-1014 added addMdcAsHeaders into documentation

GH-1014 added addMdcAsHeaders into logback appender. added integration test

GH-1014 updated documentation

GH-1014 minor fix

GH-1014 updated documentation

GH-1014 minor fix

GH-1014 removed this prefix

* Made addMdcAsHeaders true by default

* Polishing

# Conflicts:
#	spring-rabbit/src/test/java/org/springframework/amqp/rabbit/log4j2/AmqpAppenderTests.java
#	spring-rabbit/src/test/java/org/springframework/amqp/rabbit/logback/AmqpAppenderIntegrationTests.java
artembilan added a commit that referenced this issue Jun 13, 2019
Fixes #1014

**Cherry-pick to 2.1.x, 2.0.x & 1.7.x**

GH-1014 minor changes

GH-1014 renamed property to addMdcAsHeaders

GH-1014 added addMdcAsHeaders into documentation

GH-1014 added addMdcAsHeaders into logback appender. added integration test

GH-1014 updated documentation

GH-1014 minor fix

GH-1014 updated documentation

GH-1014 minor fix

GH-1014 removed this prefix

* Made addMdcAsHeaders true by default

* Polishing

# Conflicts:
#	spring-rabbit/src/test/java/org/springframework/amqp/rabbit/log4j2/AmqpAppenderTests.java
#	spring-rabbit/src/test/java/org/springframework/amqp/rabbit/logback/AmqpAppenderIntegrationTests.java

# Conflicts:
#	spring-rabbit/src/main/java/org/springframework/amqp/rabbit/log4j2/AmqpAppender.java
#	spring-rabbit/src/main/java/org/springframework/amqp/rabbit/logback/AmqpAppender.java
#	src/reference/asciidoc/logging.adoc
#	src/reference/asciidoc/whats-new.adoc
artembilan added a commit that referenced this issue Jun 13, 2019
Fixes #1014

**Cherry-pick to 2.1.x, 2.0.x & 1.7.x**

GH-1014 minor changes

GH-1014 renamed property to addMdcAsHeaders

GH-1014 added addMdcAsHeaders into documentation

GH-1014 added addMdcAsHeaders into logback appender. added integration test

GH-1014 updated documentation

GH-1014 minor fix

GH-1014 updated documentation

GH-1014 minor fix

GH-1014 removed this prefix

* Made addMdcAsHeaders true by default

* Polishing

# Conflicts:
#	spring-rabbit/src/test/java/org/springframework/amqp/rabbit/log4j2/AmqpAppenderTests.java
#	spring-rabbit/src/test/java/org/springframework/amqp/rabbit/logback/AmqpAppenderIntegrationTests.java

# Conflicts:
#	spring-rabbit/src/main/java/org/springframework/amqp/rabbit/log4j2/AmqpAppender.java
#	spring-rabbit/src/main/java/org/springframework/amqp/rabbit/logback/AmqpAppender.java
#	src/reference/asciidoc/logging.adoc
#	src/reference/asciidoc/whats-new.adoc

# Conflicts:
#	spring-rabbit/src/main/java/org/springframework/amqp/rabbit/log4j2/AmqpAppender.java
#	spring-rabbit/src/main/java/org/springframework/amqp/rabbit/logback/AmqpAppender.java
#	spring-rabbit/src/test/java/org/springframework/amqp/rabbit/log4j2/AmqpAppenderTests.java
#	spring-rabbit/src/test/java/org/springframework/amqp/rabbit/logback/AmqpAppenderIntegrationTests.java
#	spring-rabbit/src/test/resources/log4j2-amqp-appender.xml
#	spring-rabbit/src/test/resources/logback-test.xml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.