logfellow/logstash-logback-encoder

Making SingleFieldAppendingMarker#getFieldValue() package-private broke log assertions

waschmittel opened this issue · 12 comments

waschmittel commented

Description

I am maintaining https://github.com/dm-drogeriemarkt/log-capture/ which we use to test our logging.

I also allows to assert logging of keyValue, see https://github.com/dm-drogeriemarkt/log-capture/#key-value-from-logstash

Making SingleFieldAppendingMarker#getFieldValue() package-private broke that for me, because I used it for those assertions. I don't see a viable replacement, although I might be overlooking something.

Expected behavior

I am aware that is kind of a https://xkcd.com/1172/ situation ;-), but I would like a way to be still able to use those assertions. If everything else fails, I can mis-use toStringSelf() and filter out the key, but I'd rather not do that.

  • logstash-logback-encoder 7.1
elefeint commented

Was ObjectAppendingMarker.getFieldValue() never meant to be public API? We relied on it, perhaps incorrectly, to enrich logging data with Logstash markers.

brenuart commented

Hi @elefeint

The visibility of getFieldValue() has indeed been reduced - it wasn't meant to be part of the API and should ideally remain internal to the class hierarchy.

As you know, we also have more complex markers like MapEntriesAppendingMarker and ObjectFieldAppendingMarker in addition to the SingleFieldAppendingMarker... If you tell me more about your usage scenario may be we could find a more powerful approach that would give you access to the information contained in the other markers as well..

elefeint commented

The enhancer is an optional add-on that adds Logstash markers as log metadata for logs written to GCP Cloud Logging. So as long as we have a way to extract marker information from logback's ILoggingEvent, we don't need to rely on the marker's internal implementation details.
For background, GoogleCloudPlatform/spring-cloud-gcp#198 is the feature request for which this enhancer was implemented.

philsttr commented

Does spring-cloud-gcp logging allow appending arbitrary "raw" JSON to their JSON event output? If so, it could:

  1. create a JsonGenerator that writes to a string,
  2. call logstashMarker.writeTo(JsonGenerator)
  3. append the string as "raw" JSON to their JSON event output

This approach would support every type of LogstashMarker (not just the implementations of SingleFieldAppendingMarker)

elefeint commented

spring-cloud-gcp glues together Logback and GCP abilities -- in this case, logback's JsonLayout. JsonLayout accepts a map of keys and values, but not raw JSON.

philsttr commented

That's unfortunate. The limitations of JsonLayout are one of the reasons logstash-logback-encoder exists ;)

The LogstashMarkers weren't really designed to be consumed by anything other than the encoders within logstash-logback-encoder. This issue is the first case I've heard about where LogstashMarkers are being consumed by something other than the encoders within logstash-logback-encoder. As such, the current implementation in spring-cloud-gcp that consumes the markers directly is going to be extremely fragile.

Will need to think about this some to come up with a good approach.

elefeint commented

Thank you! For now, we'll hold off on upgrading.

waschmittel commented

Fwiw, we also need the actual value and not JSON for our assertions @philsttr, since the whole point of those assertions is that they are based on content and should work regardless of log formatting/encoding.

So I would also be happy if there was a way to get to the contents.

  • 👍1
waschmittel commented

Not an issue for me anymore. I've removed the assertion from my log testing library.

brenuart commented

@waschmittel Thanks for the feedback.
@elefeint Are you still impacted by this issue?

elefeint commented

Yes, but because it's an optional add-on feature, we could remove it in a major release in early 2023.
It's an understandable mismatch in targeted use-cases. Your readme does say that markers are only supported "if using LogstashEncoder/Layout"!

Ultimately, end users can create custom GCP logger enhancers with the data that they use for logstash markers.

github-actions commented

This issue has been automatically closed because there has been no response to our request for more information from the original author. With only the information that is currently in the issue, we don't have enough information to take action. Please reach out if you have or find the answers we need so that we can investigate further.