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

feat: add stats support to zpages exporter #112

Merged
merged 2 commits into from
Aug 31, 2018

Conversation

djonathascardoso
Copy link
Member

This PR adds stats support to zpages exporter. Unit tests are included.

// Fills the output columns for that method
if (view.name === DefaultViews.CLIENT_SENT_BYTES_PER_RPC ||
view.name === DefaultViews.SERVER_SENT_BYTES_PER_RPC) {
const distribution = snapshot as DistributionData;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you throw an error if it's not a DistributionData, and remove this cast? (Also in below)

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.

@@ -65,21 +77,56 @@ export class ZpagesExporter implements Exporter {
}

/**
* Is called whenever a span is started.
* It's called whenever a span is started.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Remove It's and just have it say: "Called whenver a span is started."

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

assert.equal(bucket3.lowBoundary, 30);
assert.equal(bucket3.highBoundary, 40);

// 40...Infinity)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Prefix with [

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.

@kjin kjin merged commit d38efe7 into census-instrumentation:master Aug 31, 2018
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

3 participants