Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Stats/Stackdriver: Add user agent as client header #320

Merged
merged 2 commits into from
Jan 31, 2019
+24 −10
Merged

Stats/Stackdriver: Add user agent as client header #320

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2246a61
Stats/Stackdriver: Add user agent as client header
mayurkale22 Jan 31, 2019
096641f
fix review comment
mayurkale22 Jan 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions packages/opencensus-exporter-stackdriver/src/stackdriver-monitoring.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,22 @@
* limitations under the License.
*/

import {logger, Logger, Measurement, Metric, MetricDescriptor as OCMetricDescriptor, MetricProducerManager, Metrics, StatsEventListener, TagKey, TagValue, View} from '@opencensus/core';
import {logger, Logger, Measurement, Metric, MetricDescriptor as OCMetricDescriptor, MetricProducerManager, Metrics, StatsEventListener, TagKey, TagValue, version, View} from '@opencensus/core';
import {auth, JWT} from 'google-auth-library';
import {google} from 'googleapis';

import {createMetricDescriptorData, createTimeSeriesList, getDefaultResource} from './stackdriver-stats-utils';
import {MonitoredResource, StackdriverExporterOptions, TimeSeries} from './types';

google.options({headers: {'x-opencensus-outgoing-request': 0x1}});
const OC_USER_AGENT = {
product: 'opencensus-node',
version
};
const OC_HEADER = {
'x-opencensus-outgoing-request': 0x1
};

google.options({headers: OC_HEADER});
const monitoring = google.monitoring('v3');
const GOOGLEAPIS_SCOPE = 'https://www.googleapis.com/auth/cloud-platform';

Expand Down Expand Up @@ -151,10 +160,13 @@ export class StackdriverStatsExporter implements StatsEventListener {
};

return new Promise((resolve, reject) => {
monitoring.projects.timeSeries.create(request, (err: Error) => {
this.logger.debug('sent time series', request.resource.timeSeries);
err ? reject(err) : resolve();
});
monitoring.projects.timeSeries.create(
request, {headers: OC_HEADER, userAgentDirectives: [OC_USER_AGENT]},
(err?: Error) => {
this.logger.debug(
'sent time series', request.resource.timeSeries);
err ? reject(err) : resolve();
Copy link
Contributor

Choose a reason for hiding this comment

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

Optional nit: since you are checking for the presence of err, should you have the parameter be err?: Error?

(Although the package doesn't have strict null checks enabled, I think it's still helpful to think in terms of them and then if eventually we want to turn them on they will be easier to enable).

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

});
});
});
}
Expand All @@ -173,10 +185,12 @@ export class StackdriverStatsExporter implements StatsEventListener {
};

return new Promise((resolve, reject) => {
monitoring.projects.metricDescriptors.create(request, (err: Error) => {
this.logger.debug('sent metric descriptor', request.resource);
err ? reject(err) : resolve();
});
monitoring.projects.metricDescriptors.create(
request, {headers: OC_HEADER, userAgentDirectives: [OC_USER_AGENT]},
Copy link
Contributor

Choose a reason for hiding this comment

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

Will you need similar logic for other types of Google API calls in the exporter, e.g. writing traces?

If so, what would you think about exporting the larger {headers: OC_HEADER, userAgentDirectives: [OC_USER_AGENT]} constant as something like OC_HEADER_OPTIONS or similar and making it be in a shared utility between trace and metrics?

Copy link
Member Author

Choose a reason for hiding this comment

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

As per my understanding, User Agent requirement is only Stackdriver stats exporter specific. For Stackdriver Trace exporter, we add agent information as a part of the label (example: g.co/agent -> opencensus-node [{version}]). This is implemented in all the libraries except node. Issue #39 -> to address that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, makes sense, then keeping these constants in this file only seems fine.

(err?: Error) => {
this.logger.debug('sent metric descriptor', request.resource);
err ? reject(err) : resolve();
});
});
});
}
Expand Down