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: Write full message into short_message field #34

Open
wants to merge 1 commit into
base: v2
Choose a base branch
from

Conversation

valentin-krasontovitsch
Copy link

Following the principle of least surprise, we write messages completely
into the short_message field, even if they contain several lines.

To not break gelf standard, we also write the full message, always, to
the full_message field.

Resolves #11

Following the principle of least surprise, we write messages completely
into the `short_message` field, even if they contain several lines.

To not break gelf standard, we also write the full message, always, to
the `full_message` field.

Resolves Graylog2#11
@mariussturm
Copy link

Sorry I still don't get what problem you are trying to solve here :/
With this change you would force every user to have the full log message in two fields, right? So that the data that is transferred becomes double the size. Usually you keep a library as flexible as possible and let the user decide what to do with it. So when a user wants that behaviour she can easily fill both fields in the client code. There is no need to force it in the library. Maybe you can explain a bit more what you want to achieve?

@valentin-krasontovitsch
Copy link
Author

Okay well so I mean I dunno this was a long time ago 😅

Let me try to catch up and understand, I was and perhaps still am a bit confused about all this...

It seems like part of the discussion is in issue #11 - and there you indeed say

But in the end using only the message field should be what most people expect.

I think that's why I decided to go ahead and remove the full message field.

But - that's not up to spec, which is a very good point.

SO, then I went ahead and did the (what I thought) reasonable thing of putting the whole message by default into both short and full message field.

I'm lazy by nature, so I didn't want to have to compose my own message struct every time... I just wanted to have one function in this lib that I could use and feed with the message I wanted to send.

Now I see of course that doubling the amount of data is not convenient.
Perhaps the "right" behaviour (according to our opinion, of course) for the Write([]byte) method is to write the full message into the short message field, which becomes the message field in the greylog server and is what most people would like to look at (exclusively), and simply leave the full message blank.

That way, "default" lazy behaviour is what I imagine to be that of least surprise, and we don't get double data, and people can still do whatever they want by using the WriteMessage(*Message) method.

Does that sound good?

@mariussturm
Copy link

Okay, I see what you mean. We could consider the proposed change but this would defiantly be a breaking change so we would need a new version branch for this.

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