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

pin dependency awesome_print to v1.7.0 to avoid bug in v1.8.0 #5

Merged

Conversation

yaauie
Copy link
Contributor

@yaauie yaauie commented Jun 4, 2018

The awesome_print release v1.8.0 contains a bug that causes the Logstash
process to crash while loading if an exception is raised while the library
attempts to load its user-defined defaults from disk (this can occur if the
HOME environment variable is unset, or if the JVM fails to read the file
at ${HOME}/.aprc).

While the bug has been fixed on the awesome_print master branch, until
a release is rolled, pin our dependency to an earlier version to avoid the
bug.

A release of the fix upstream has been requested on 2018-05-02 via
awesome-print/awesome_print#338

@jakelandis
Copy link
Contributor

It seems the difference between 1.7.0 and 1.8.0 is pretty large. Pinning to the elder version may fix this issue, but it also may introduce other issues that may have been fixed:

awesome-print/awesome_print@v1.7.0...v1.8.0

There is outstanding issue in the stdout plugin with workaround:

logstash-plugins/logstash-output-stdout#11

This only happens when run as a service and trying to use this debug. I would prefer to keep the version un-pinned and refer people to work around until the upstream gets fixed.

@guyboertje
Copy link

I went through all the changes between 1.7.0 and 1.8.0.
There are plenty of changes to the internal code as cleanup.
There are some changes to formatters for Sequel, Rails and Object instances - none of which apply to our constrained set to allowable Event datatypes.
There are some changes to the formatters for a Ruby Hash but these display using the key: value format which is less familiar than the key => value format as recognised from the LS config.

My opinion is: its OK to pin this gem to 1.7.0 as long as we have a plan to replace awesome_print with our own formatter.

@guyboertje guyboertje requested review from guyboertje and removed request for jakelandis July 17, 2018 06:36
Copy link

@guyboertje guyboertje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - I think the risks are few in doing this.

@guyboertje
Copy link

Another rationale...
LS 2.X up to 5.4.1 used awesome_print v 1.7.0 without issue and the Event became constrained at v5.

The `awesome_print` release v1.8.0 contains a bug that causes the Logstash
process to crash while loading if an exception is raised while the library
attempts to load its user-defined defaults from disk (this can occur if the
`HOME` environment variable is unset, or if the JVM fails to read the file
at `${HOME}/.aprc`).

While the bug has been [fixed][] on the `awesome_print` master branch, until
a release is rolled, pin our dependency to an earlier version to avoid the
bug.

[fixed]: awesome-print/awesome_print@62033856f8ff083
@yaauie yaauie force-pushed the pin-dependency-to-avoid-bug branch from 329002c to d5bfc82 Compare July 18, 2018 20:42
@guyboertje guyboertje merged commit b57ccec into logstash-plugins:master Jul 27, 2018
@imajes
Copy link

imajes commented Jan 23, 2019

Hey guys! I know this is merged etc, but if you still use ap for formatting in logstash, it'd be great to see how version 2.0.0.pre works out for you! :D

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.

5 participants