Allow turning on JSON Detection in StackDriver #6293

yuvipanda · Oct 23, 2018

To enable JSON detection in StackDriver, jsonPayload should
have no fields other than message. This patch lets clients
opt-in to this behavior, by creating a custom Transport.

Fixes #5799

logging/google/cloud/logging/handlers/transports/background_thread.py

+        }
+
+        if self._include_logger_name:
+            log_record['info']['python_logger'] = record.name 


logging/google/cloud/logging/handlers/transports/background_thread.py

@@ -230,7 +236,7 @@ def _main_thread_terminated(self):
                file=sys.stderr)

    def enqueue(self, record, message, resource=None, labels=None,
-                trace=None, span_id=None):
+                trace=None, span_id=None, include_logger_name=True):


tseaver

We need test coverage for the case where the user disables adding the logger name.

To enable JSON detection in StackDriver, `jsonPayload` should have no fields other than `message`. This patch lets clients opt-in to this behavior, by creating a custom Transport. Fixes #5799

yuvipanda · Oct 24, 2018

Thanks for the comments, @crwilcox! Fixed.

@tseaver will add those shortly.

yuvipanda · Oct 24, 2018

@tseaver added tests and they seem to pass!

This patch sends events (currently, just launches) to stackdriver. The events aren't parsed as JSON by stackdriver yet, but that should be fixed by googleapis/google-cloud-python#6293. You can find the events under 'Global -> binderhub-events-text'.

yuvipanda · Oct 25, 2018

@crwilcox I found linting errors from @kokoro-team, and pushed a fix! Thanks

yuvipanda · Oct 25, 2018

I missed one lint failure, fixed now. Would be easier if lint ran on CircleCI or something earlier.

crwilcox · Oct 25, 2018

Hi @yuvipanda . Thanks for the PR. Something 'funny' is going on with the test runs on GitHub right now. You should see a series of tests happening for this, but none are showing. We will need to sort that out before merging.

yuvipanda · Oct 26, 2018

@crwilcox are these the @kokoro-team tests or something else? Anything I can do?

logging/tests/unit/handlers/transports/test_background_thread.py

@@ -166,15 +166,17 @@ def test_constructor(self):
        grace_period = 50
        max_batch_size = 50
        max_latency = 0.1
+        include_logger_name = True


logging/tests/unit/handlers/transports/test_background_thread.py


        worker = self._make_one(
            logger, grace_period=grace_period, max_batch_size=max_batch_size,
-            max_latency=max_latency)
+            max_latency=max_latency, include_logger_name=include_logger_name)


logging/tests/unit/handlers/transports/test_background_thread.py


        self.assertEqual(worker._cloud_logger, logger)
        self.assertEqual(worker._grace_period, grace_period)
        self.assertEqual(worker._max_batch_size, max_batch_size)
        self.assertEqual(worker._max_latency, max_latency)
+        self.assertEqual(worker._include_logger_name, include_logger_name)


logging/tests/unit/handlers/transports/test_background_thread.py

tseaver · Oct 26, 2018

@crwilcox I believe your comments have been addressed. CI is green, so merge when ready.

crwilcox · Oct 26, 2018

Thank you @yuvipanda for the contribution!

yuvipanda · Oct 27, 2018

Awesome! Thank you very much, @crwilcox and @tseaver. Do you know when the next release to PyPI will be?

Includes googleapis/google-cloud-python#6293. Lets us push structured logs to stackdriver, rather than textual logs.

This reverts commit d72f443. (PR #6293)

yuvipanda requested a review from crwilcox as a code owner October 23, 2018 21:11

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 23, 2018

crwilcox suggested changes Oct 24, 2018

View reviewed changes

logging/google/cloud/logging/handlers/transports/background_thread.py Outdated

}

if self._include_logger_name:

log_record['info']['python_logger'] = record.name

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

crwilcox reviewed Oct 24, 2018

View reviewed changes

tseaver suggested changes Oct 24, 2018

View reviewed changes

Allow turning on JSON Detection in StackDriver

b24f7a4

To enable JSON detection in StackDriver, `jsonPayload` should have no fields other than `message`. This patch lets clients opt-in to this behavior, by creating a custom Transport. Fixes #5799

Add tests for include_python_logger

e8fc063

yuvipanda mentioned this pull request Oct 25, 2018

Send eventlogs to StackDriver jupyterhub/mybinder.org-deploy#787

Merged

crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 25, 2018

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 25, 2018

tseaver added api: logging Issues related to the Cloud Logging API. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Oct 25, 2018

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 25, 2018

Fix lint errors

653848b

tseaver suggested changes Oct 26, 2018

View reviewed changes

crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 26, 2018

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 26, 2018

Cleanup tests for background_thread logging provider

22d80e2

tseaver added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 26, 2018

tseaver approved these changes Oct 26, 2018

View reviewed changes

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 26, 2018

crwilcox approved these changes Oct 26, 2018

View reviewed changes

crwilcox merged commit d72f443 into googleapis:master Oct 26, 2018

yuvipanda deleted the patch-1 branch October 27, 2018 04:15

yuvipanda added a commit to yuvipanda/binderhub that referenced this pull request Oct 29, 2018

Upgrade version of google-cloud-logging

6e91396

Includes googleapis/google-cloud-python#6293. Lets us push structured logs to stackdriver, rather than textual logs.

This was referenced Oct 29, 2018

Upgrade version of google-cloud-logging jupyterhub/binderhub#706

Merged

Logging Handlers add the attribute python_logger which breaks the stackdriver convention for jsonpayload #5799

Closed

tseaver mentioned this pull request Oct 31, 2018

Revert "Allow turning on JSON Detection in StackDriver" #6352

Merged

tseaver added a commit that referenced this pull request Oct 31, 2018

Revert "Allow turning on JSON Detection in StackDriver" (#6352)

c6bc48c

This reverts commit d72f443. (PR #6293)

Search code, repositories, users, issues, pull requests...

Allow turning on JSON Detection in StackDriver #6293

Allow turning on JSON Detection in StackDriver #6293

Uh oh!

Conversation

yuvipanda commented Oct 23, 2018

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

tseaver left a comment

Choose a reason for hiding this comment

Uh oh!

yuvipanda commented Oct 24, 2018

Uh oh!

yuvipanda commented Oct 24, 2018

Uh oh!

yuvipanda commented Oct 25, 2018

Uh oh!

yuvipanda commented Oct 25, 2018

Uh oh!

crwilcox commented Oct 25, 2018

Uh oh!

yuvipanda commented Oct 26, 2018

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

Uh oh!

Uh oh!

Uh oh!

tseaver commented Oct 26, 2018

Uh oh!

crwilcox commented Oct 26, 2018

Uh oh!

yuvipanda commented Oct 27, 2018

Uh oh!

Uh oh!