Add default resource generator for log entries #1808

cristiancavalli · 2016-11-20T09:37:05Z

To Dos

Add environment detection helpers to google-auto-auth: add environment detection helpers stephenplusplus/google-auto-auth#8
Update google-cloud/common with new google-auto-auth release
Tests
- System
- Unit
  - log.js
  - metadata.js

To accommodate invocations like:
syslog.entry({textPayload: 'test'})
Create a basic class which attempts to automatically
establish resource values based on connectivity to
the metadata service and environmental variables.

This addition does not affect the existing API and
does not modify or augment user supplied resources.
The meta data class will attempt to determine if the
resource is a cloud function, app engine application
or compute engine instance. If it can't match on any
of these configurations but can still locate a project
id then it will produce a global resource id.

This addition adds behavioral tests and adds a new
Metadata class-specific set of unit tests which reach
100% module coverage:
https://coveralls.io/github/cristiancavalli/google-cloud-node

cc @ofrobots

packages/logging/src/log.js

 };

+Log.prototype.assignDefaultMetadata_ = function(entry, callback) {
+  var entryMetadata = entry.metadata;


packages/logging/src/metadata.js

+ * @returns {module:metadata}
+ * @chainable
+ */
+Metadata.prototype.getProjectId = function(callback) {


packages/logging/src/metadata.js

+ * @private
+ */
+Metadata.prototype.attachRequestStreamListeners_ = function(stream) {
+  var self = this;


packages/logging/src/metadata.js

+      'Must supply a callback function as an argument to invocation.');
+  } else if (!is.empty(self.processProjectId_()) &&
+    is.string(self.processProjectId_())) {
+    // The process's GCLOUD_PROJECT env variable has been set


packages/logging/src/log.js

+  if (!is.object(entryMetadata) || !is.object(entryMetadata.resource)) {
+    this.metadata_.getDefaultResource(function(err, resource) {
+      if (err) {
+        callback(err);


stephenplusplus · 2016-11-21T13:26:02Z

I noticed this PR doesn't decorate requests sent through the write method, but only ones the severity shortcuts. Is this intentional?

cristiancavalli · 2016-11-21T18:00:30Z

refactored the call chain to only affect the write method

packages/logging/src/metadata.js

+    labels: {
+      project_id: projectId,
+      module_id: process.env.GAE_SERVICE,
+      version_id: process.env.GAE_VERSION


packages/logging/src/log.js

+  var self = this;
+  this.metadata_.getDefaultResource(function(err, resource) {
+    if (err) {
+      callback(err);


stephenplusplus · 2016-11-28T17:18:49Z

@cristiancavalli can you try this out on the various environments to see that it's working as intended? I've made a small zip you can run (edit the env object in system-test/logging.js, npm install && npm run system-test after installing), https://storage.googleapis.com/stephen-has-a-new-bucket/pr-1808-test.zip

cristiancavalli · 2016-11-30T04:08:53Z

@stephenplusplus Sorry for the latency -- just finished testing on GCE and all tests pass. This passing test should apply to GKE as well.

stephenplusplus · 2016-11-30T16:19:08Z

This is ready for a final review. @callmehiphop PTAL.

packages/logging/src/log.js

-  var formattedEntry = entry.toJSON();
-  formattedEntry.logName = this.formattedName_;
-  return formattedEntry;
+  async.map(entries, function(entry, callback) {


callmehiphop · 2016-11-30T20:16:18Z

@stephenplusplus I think we need to stub the auth client or something, I'm seeing documentation failures in regards to grabbing the projectId.

stephenplusplus · 2016-11-30T20:52:15Z

Pushed a fix that's a little bit sad. Let me know if you know a better way.

coveralls · 2016-12-01T11:34:01Z

Changes Unknown when pulling 66f67b0 on cristiancavalli:automatic-metadata into ** on GoogleCloudPlatform:master**.

callmehiphop · 2016-12-01T19:42:54Z

@stephenplusplus I'm ok with that if you are. Alternatively I think we could just stub google-auto-auth in the docs spec file. Something like..

var FakeAuth = function() {
  var Auth = require('google-auto-auth');
  Auth.prototype.getProjectId = function(cb) {
    cb(null, 'grape-spaceship-123');
  };
  return Auth;
};

stephenplusplus · 2016-12-01T19:46:33Z

I went that way originally, but couldn't get it to work :( It's like it's never required directly in our snippet tests. Let me know if you know a better way to do it than:

--- a/test/docs.js
+++ b/test/docs.js
@@ -57,6 +57,14 @@ var JSHintError = createErrorClass('JSHintError', function(err) {
   this.code = err.code;
 });

+var FakeAuth = function() {
+  var Auth = require('google-auto-auth');
+  Auth.prototype.getProjectId = function(cb) {
+    cb(null, 'grape-spaceship-123');
+  };
+  return Auth;
+};
+
 var FakeConsole = Object.keys(console)
   .reduce(function(console, method) {
     console[method] = noop;
@@ -245,6 +253,7 @@ function createSnippet(mod, instantiation, method) {
       'keyFilename: \'/path/to/keyfile.json\'',
       'keyFilename: \'\''
     )
+    .replace('require(\'google-auto-auth\')', FakeAuth.toString())
     .replace('require(\'express\')', FakeExpress.toString())
     .replace('require(\'level\')', FakeLevel.toString())
     .replace('require(\'bluebird\')', FakeBluebird.toString())

callmehiphop · 2016-12-01T20:02:09Z

Ah, right. We only swap out dependencies in the examples themselves.. I think if we wanted to go this route we'd probably have to run some code in the sandbox that uses proxyquire. So I think this is fine for now and maybe I'll try and take a crack at implementing proxyquire later.

callmehiphop · 2016-12-01T20:34:27Z

Can we add logging/metadata.js to the ignore list within scripts/docs/config.js?

To accommodate invocations like: `syslog.entry({textPayload: 'test'})` Create a basic class which attempts to automatically establish resource values based on connectivity to the metadata service and environmental variables. This addition does not affect the existing API and does not modify or augment user supplied resources. The meta data class will attempt to determine if the resource is a cloud function, app engine application or compute engine instance. If it can't match on any of these configurations but can still locate a project id then it will produce a global resource id.

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 20, 2016

cristiancavalli force-pushed the automatic-metadata branch 14 times, most recently from 31b483e to 03ef83d Compare November 20, 2016 21:10

stephenplusplus added enhancement api: logging Issues related to the Cloud Logging API. labels Nov 21, 2016

stephenplusplus reviewed Nov 21, 2016

View reviewed changes

cristiancavalli force-pushed the automatic-metadata branch 2 times, most recently from d19f782 to e326dad Compare November 21, 2016 17:57

ofrobots reviewed Nov 21, 2016

View reviewed changes

cristiancavalli force-pushed the automatic-metadata branch 3 times, most recently from 814c95a to 285cb40 Compare November 21, 2016 22:12

ofrobots reviewed Nov 21, 2016

View reviewed changes

packages/logging/src/log.js Outdated

var self = this;

this.metadata_.getDefaultResource(function(err, resource) {

if (err) {

callback(err);

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

cristiancavalli force-pushed the automatic-metadata branch from 285cb40 to b789a1a Compare November 22, 2016 00:54

stephenplusplus mentioned this pull request Nov 22, 2016

add environment detection helpers stephenplusplus/google-auto-auth#8

Merged

googlebot removed the cla: yes This human has signed the Contributor License Agreement. label Nov 22, 2016

ofrobots mentioned this pull request Nov 28, 2016

Add winston logging transport #1830

Merged

stephenplusplus force-pushed the automatic-metadata branch 2 times, most recently from 9ddc84c to b347b54 Compare November 30, 2016 16:18

stephenplusplus removed the don't merge label Nov 30, 2016

stephenplusplus assigned callmehiphop and unassigned stephenplusplus Nov 30, 2016

callmehiphop suggested changes Nov 30, 2016

View reviewed changes

callmehiphop approved these changes Nov 30, 2016

View reviewed changes

cristiancavalli and others added 10 commits December 2, 2016 08:12

detect environment with google-auto-auth

d932c61

remove duplexify as dependency

f0c6ea6

correct method call

ab911aa

add system test

9081bef

add tests for log.js

a77decf

test metadata.js

2c9915d

move async to dependencies

991a1b4

fix snippet test

cbac7be

add logging/metadata to docs ignore

691d385

stephenplusplus force-pushed the automatic-metadata branch from 66f67b0 to 691d385 Compare December 2, 2016 13:12

stephenplusplus merged commit 978b1f5 into googleapis:master Dec 2, 2016

Add default resource generator for log entries #1808

Add default resource generator for log entries #1808

Uh oh!

Conversation

cristiancavalli commented Nov 20, 2016 • edited by stephenplusplus Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

To Dos

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!

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!

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!

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!

stephenplusplus commented Nov 21, 2016

Uh oh!

cristiancavalli commented Nov 21, 2016

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!

This comment was marked as spam.

Uh oh!

stephenplusplus commented Nov 28, 2016

Uh oh!

cristiancavalli commented Nov 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stephenplusplus commented Nov 30, 2016

Uh oh!

This comment was marked as spam.

Uh oh!

This comment was marked as spam.

cristiancavalli commented Nov 20, 2016 •

edited by stephenplusplus

Loading

cristiancavalli commented Nov 30, 2016 •

edited

Loading

stephenplusplus commented Dec 1, 2016 •

edited

Loading