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 #24

Conversation

valentin-krasontovitsch

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

As a consequence, we deprecate the full_message field, as it becomes
redundant.

Resolves #11

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

As a consequence, we deprecate the `full_message` field, as it becomes
redundant.

Resolves Graylog2#11
@valentin-krasontovitsch
Copy link
Author

Since this is an API change (functionality of Write method is changed), it should probably be merged into a new v3 branch.

If you accept the merge request for more precise time stamps first, I'll be happy to rebase this guy on top of that.

@alexbusu
Copy link

Thanks for adding this. What if we can consider this PR a patch (like, for a bug fix), and let the merge in v2? 😄

@mariussturm
Copy link

@valentin-krasontovitsch thanks again for this PR! I would not simply remove the full_message field as it is still part of the GELF specification: https://docs.graylog.org/en/3.1/pages/gelf.html
Maybe a future version of GELF will sort this out but currently we should follow the specs. The full_message field is currently optional so why not just leave it there and use short_message like you described?

@valentin-krasontovitsch
Copy link
Author

Okay finally coming around to looking at this again - sorry for the incredible delay...

Closing in favor of #34, which writes the whole message into both fields, period - hope this is what' desired ^^

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.

3 participants