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

Adds metric name prefix override for [exporter-stackdriver] #151

Conversation

isaikevych
Copy link
Contributor

  • The metric type, including its DNS name prefix. The type is not URL-encoded.
    All user-defined metric types have the DNS name custom.googleapis.com or
    external.googleapis.com. Metric types should use a natural hierarchical grouping.
    For example: ""custom.googleapis.com/invoice/paid/amount""
    ""external.googleapis.com/prometheus/up""
    ""appengine.googleapis.com/http/server/response_latencies"""

@songy23
Copy link
Contributor

songy23 commented Oct 17, 2018

Please fix the build.

@songy23
Copy link
Contributor

songy23 commented Oct 17, 2018

Error log:

src/stackdriver-monitoring.ts:72:52 - error TS2339: Property 'DELAY' does not exist on type 'typeof StackdriverStatsExporter'.
  setTimeout(resolve, StackdriverStatsExporter.DELAY);

Looks like this PR depends on #150.

@isaikevych
Copy link
Contributor Author

isaikevych commented Oct 17, 2018

Error log:

src/stackdriver-monitoring.ts:72:52 - error TS2339: Property 'DELAY' does not exist on type 'typeof StackdriverStatsExporter'.
  setTimeout(resolve, StackdriverStatsExporter.DELAY);

Looks like this PR depends on #150.

This pull request is blocked by #150

@isaikevych isaikevych force-pushed the stackdriver_metric-name-prefix_override branch 3 times, most recently from 9dc866e to d1eaedb Compare October 18, 2018 00:04
@isaikevych isaikevych requested a review from kjin October 18, 2018 00:16
Copy link
Contributor

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -42,6 +42,8 @@ export interface StackdriverExporterOptions extends ExporterConfig {
* projectId project id defined to stackdriver
*/
projectId: string;
/** Prefix for metric */
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a note on when the user might want to set this, and when they don't need to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@isaikevych isaikevych force-pushed the stackdriver_metric-name-prefix_override branch 5 times, most recently from 485763e to 1307240 Compare October 18, 2018 01:33
Copy link
Contributor

@kjin kjin left a comment

Choose a reason for hiding this comment

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

LGTM once comments are addressed

/**
* Gets metric type
* @param name The view name
* @param prefix Optional metric prefix
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@isaikevych isaikevych force-pushed the stackdriver_metric-name-prefix_override branch 3 times, most recently from be5cfb5 to 00f17cf Compare October 18, 2018 18:36
@isaikevych isaikevych force-pushed the stackdriver_metric-name-prefix_override branch from 00f17cf to c83203a Compare October 18, 2018 18:53
@isaikevych isaikevych merged commit 869ce34 into census-instrumentation:master Oct 18, 2018
@isaikevych
Copy link
Contributor Author

Fixes #125

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants