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

Commit

Permalink
fix review comments
Browse files Browse the repository at this point in the history
1. Remove map -> constantLabels
2. Make labels values and keys merging code consistent.
3. Update error message
  • Loading branch information
mayurkale22 committed Apr 4, 2019
1 parent 9680131 commit 1b0dd33
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 14 deletions.
12 changes: 5 additions & 7 deletions packages/opencensus-core/src/common/validations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,11 @@ export function validateMapElementNotNull<T>(

/** Throws an error if any of the array element present in the map. */
export function validateDuplicateKeys(
keys: LabelKey[], map: Map<LabelKey, LabelValue>) {
const keyNames: Set<string> = new Set();
keys.forEach(key => keyNames.add(key.key));
Array.from(map.keys()).forEach(key => keyNames.add(key.key));

if (keyNames.size !== (keys.length + map.size)) {
keys: LabelKey[], constantLabels: Map<LabelKey, LabelValue>) {
const keysAndConstantKeys =
new Set([...keys, ...constantLabels.keys()].map(k => k.key));
if (keysAndConstantKeys.size !== (keys.length + constantLabels.size)) {
throw new Error(
`The keys from LabelKeys should not be present in constantLabels`);
`The keys from LabelKeys should not be present in constantLabels or LabelKeys should not contains duplicate keys`);
}
}
4 changes: 2 additions & 2 deletions packages/opencensus-core/src/metrics/gauges/derived-gauge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ export class DerivedGauge implements types.Meter {
type: MetricDescriptorType, labelKeys: LabelKey[],
readonly constantLabels: Map<LabelKey, LabelValue>) {
this.labelKeysLength = labelKeys.length;
const combinedLabelKeys = labelKeys.concat(...constantLabels.keys());
const keysAndConstantKeys = [...labelKeys, ...constantLabels.keys()];
this.constantLabelValues = [...constantLabels.values()];

this.metricDescriptor =
{name, description, unit, type, labelKeys: combinedLabelKeys};
{name, description, unit, type, labelKeys: keysAndConstantKeys};
}

// Checks if the specified collection is a LengthAttributeInterface.
Expand Down
4 changes: 2 additions & 2 deletions packages/opencensus-core/src/metrics/gauges/gauge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ export class Gauge implements types.Meter {
type: MetricDescriptorType, readonly labelKeys: LabelKey[],
readonly constantLabels: Map<LabelKey, LabelValue>) {
this.labelKeysLength = labelKeys.length;
const combinedLabelKeys = labelKeys.concat(...constantLabels.keys());
const keysAndConstantKeys = [...labelKeys, ...constantLabels.keys()];
this.constantLabelValues = [...constantLabels.values()];

this.metricDescriptor =
{name, description, unit, type, labelKeys: combinedLabelKeys};
{name, description, unit, type, labelKeys: keysAndConstantKeys};
this.defaultLabelValues = initializeDefaultLabels(this.labelKeysLength);
}

Expand Down
1 change: 0 additions & 1 deletion packages/opencensus-core/src/metrics/metric-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import {validateArrayElementsNotNull, validateDuplicateKeys, validateMapElementNotNull, validateNotNull} from '../common/validations';
import {MeasureUnit} from '../stats/types';

import {BaseMetricProducer} from './export/base-metric-producer';
import {Metric, MetricDescriptorType, MetricProducer} from './export/types';
import {DerivedGauge} from './gauges/derived-gauge';
Expand Down
4 changes: 2 additions & 2 deletions packages/opencensus-core/test/test-metric-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describe('addInt64Gauge', () => {
const labelKeys = [{key: 'k1', description: 'desc'}];
assert.throws(() => {
registry.addInt64Gauge(METRIC_NAME, {constantLabels, labelKeys});
}, /^Error: The keys from LabelKeys should not be present in constantLabels$/);
}, /^Error: The keys from LabelKeys should not be present in constantLabels or LabelKeys should not contains duplicate keys$/);
});
});

Expand Down Expand Up @@ -273,7 +273,7 @@ describe('addDerivedInt64Gauge', () => {
assert.throws(() => {
registry.addDerivedInt64Gauge(
METRIC_NAME, {constantLabels, labelKeys});
}, /^Error: The keys from LabelKeys should not be present in constantLabels$/);
}, /^Error: The keys from LabelKeys should not be present in constantLabels or LabelKeys should not contains duplicate keys$/);
});

it('should throw an error when the constant labels elements are null', () => {
Expand Down

0 comments on commit 1b0dd33

Please sign in to comment.