-
Notifications
You must be signed in to change notification settings - Fork 227
Update LoggingHandler to stringify record.msg #312
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
Conversation
Are the build failures caused by this PR? |
Unfortunately, the builds on Travis and AppVeyor are somewhat broken at the moment. I hope to fix that soon. apm-ci is the most comprehensive build, so if that passes we should be OK. A quick note on branches. Master has a bunch of backwards incompatible changes, and will be released as 4.0 together with APM Server 6.5. We will continue to cut maintenance releases of the 3.x line for some time. I can create a new 3.x with a backport of this PR some time next week (I'm traveling at the moment). Sorry for dropping the ball on this issue, and thanks @axw for picking it up! |
I've targeted 3.x (targeting master was a mistake), so that @jaap3 can test. There are failures in apm-ci, but they appear to have something to do with json-schema; nothing I touched. |
@axw I've installed your branch, I'll keep it running for a while and report back if I see anything (earliest monday) |
@beniwohli this is probably not the best place to discuss this, but, after the release of APM Server 6.5/Client 4.0 what would an upgrade path look like? e.g. should all projects be upgraded to client version 4.0 immediately? Or can the server 6.5 handle messages from Client < 4.0? Or, inverted, can projects be safely upgraded to client 4.0 while the server is still < 6.5? This is not a huge issue for me currently, but in the future I will probably have many project reporting to the same APM server. So I'm very interested in a smooth upgrade path where I don't have to worry about updating multiple things simultaneously. From reading the CHANGELOG it seems that I should at least be pinning the client to <4 for now. |
@jaap3 regarding upgrading: APM Server 6.5 (and all coming 6.x versions) will have support for the v1 API, which the agent versions < 4.0 talk. So you should be able to upgrade your APM Server(s) first, and then upgrade the agents. The other way around is unfortunately not possible, as it would have been way too complicated to support both v1 and the new v2 API in the agent. Generally speaking, we highly suggest to pin the elastic-apm dependency to at least the major version. New major versions usually indicate a backwards incompatible change. We should probably add a note to the install docs about this... |
Update LoggingHandler so that it converts the log record message to a string. The message may be any arbitrary object, and currently non-string messages are being sent as objects, which the APM Server rejects.
785c21c
to
5852623
Compare
@axw force-pushed a rebase on top of updated 3.x branch to your repo, hope that's ok 😊 will merge as soon as tests pass (travis ci will not pass due to unrelated breakage with django master) |
Sure is. Thanks for taking this over. |
* Update LoggingHandler to stringify record.msg Update LoggingHandler so that it converts the log record message to a string. The message may be any arbitrary object, and currently non-string messages are being sent as objects, which the APM Server rejects. * added stringification to Logbook handler as well
Update LoggingHandler so that it converts the log record
message to a string. The message may be any arbitrary object,
and currently non-string messages are being sent as objects,
which the APM Server rejects.
Fixes #295