This repository has been archived by the owner on Oct 3, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 97
Add MetricProducerManager to keep track of all MetricProducers #253
Merged
mayurkale22
merged 5 commits into
census-instrumentation:master
from
mayurkale22:MetricProducerManager
Jan 7, 2019
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
aa04fe7
Add MetricProducerManager to keep set of MetricProducer
mayurkale22 9a70667
fix review comments
mayurkale22 b232d00
fix review comments
mayurkale22 88b833d
Fix JSDoc and change copyright 2018 -> 2019
mayurkale22 96c5bba
Fix JSDoc comments
mayurkale22 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
69 changes: 69 additions & 0 deletions
69
packages/opencensus-core/src/metrics/export/metric-producer-manager.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
/** | ||
* Copyright 2019, OpenCensus Authors | ||
* | ||
* Licensed under the Apache License, Version 2.0 the "License"; | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import {validateNotNull} from '../../common/validations'; | ||
import {MetricProducer, MetricProducerManager} from './types'; | ||
|
||
/** | ||
* Keeps a set of MetricProducer that is used by exporters to determine the | ||
* metrics that need to be exported. | ||
*/ | ||
class BaseMetricProducerManager implements MetricProducerManager { | ||
private metricProducers: Set<MetricProducer> = new Set<MetricProducer>(); | ||
|
||
/** | ||
* Adds the MetricProducer to the manager if it is not already present. | ||
* | ||
* @param {MetricProducer} The MetricProducer to be added to the manager. | ||
*/ | ||
add(metricProducer: MetricProducer): void { | ||
validateNotNull(metricProducer, 'metricProducer'); | ||
if (!this.metricProducers.has(metricProducer)) { | ||
this.metricProducers.add(metricProducer); | ||
} | ||
} | ||
|
||
/** | ||
* Removes the MetricProducer to the manager if it is present. | ||
* | ||
* @param {MetricProducer} The MetricProducer to be removed from the manager. | ||
*/ | ||
remove(metricProducer: MetricProducer): void { | ||
validateNotNull(metricProducer, 'metricProducer'); | ||
this.metricProducers.delete(metricProducer); | ||
} | ||
|
||
/** | ||
* Clears all MetricProducers. | ||
*/ | ||
removeAll(): void { | ||
this.metricProducers.clear(); | ||
} | ||
|
||
/** | ||
* Returns all registered MetricProducers that should be exported. | ||
* | ||
* This method should be used by any metrics exporter that automatically | ||
* exports data for MetricProducer registered with the MetricProducerManager. | ||
* | ||
* @return {Set<MetricProducer>} The Set of MetricProducers. | ||
*/ | ||
getAllMetricProducer(): Set<MetricProducer> { | ||
return this.metricProducers; | ||
} | ||
} | ||
|
||
export const metricProducerManagerInstance = new BaseMetricProducerManager(); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
/** | ||
* Copyright 2019, OpenCensus Authors | ||
* | ||
* Licensed under the Apache License, Version 2.0 the "License"; | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import {metricProducerManagerInstance} from './export/metric-producer-manager'; | ||
import {MetricRegistry} from './metric-registry'; | ||
|
||
/** | ||
* Class that holds the implementation instance for MetricRegistry. | ||
*/ | ||
export class MetricsComponent { | ||
private metricRegistry: MetricRegistry; | ||
|
||
constructor() { | ||
this.metricRegistry = new MetricRegistry(); | ||
|
||
// Register the MetricRegistry's MetricProducer to the global | ||
// MetricProducerManager. | ||
metricProducerManagerInstance.add(this.metricRegistry.getMetricProducer()); | ||
} | ||
|
||
/** | ||
* Returns the MetricRegistry. | ||
* | ||
* @return {MetricRegistry}. | ||
*/ | ||
getMetricRegistry(): MetricRegistry { | ||
return this.metricRegistry; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
/** | ||
* Copyright 2019, OpenCensus Authors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import * as assert from 'assert'; | ||
import {metricProducerManagerInstance} from '../src/metrics/export/metric-producer-manager'; | ||
import {MetricsComponent} from '../src/metrics/metric-component'; | ||
import {MetricRegistry} from '../src/metrics/metric-registry'; | ||
|
||
describe('MetricsComponent()', () => { | ||
const metricsComponent: MetricsComponent = new MetricsComponent(); | ||
|
||
it('should return a MetricRegistry instance', () => { | ||
assert.ok(metricsComponent.getMetricRegistry() instanceof MetricRegistry); | ||
}); | ||
|
||
it('should register metricRegistry to MetricProducerManger', () => { | ||
assert.equal(metricProducerManagerInstance.getAllMetricProducer().size, 1); | ||
assert.ok(metricProducerManagerInstance.getAllMetricProducer().has( | ||
metricsComponent.getMetricRegistry().getMetricProducer())); | ||
}); | ||
}); |
91 changes: 91 additions & 0 deletions
91
packages/opencensus-core/test/test-metric-producer-manager.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
/** | ||
* Copyright 2019, OpenCensus Authors | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
import * as assert from 'assert'; | ||
import {metricProducerManagerInstance} from '../src/metrics/export/metric-producer-manager'; | ||
import {MetricRegistry} from '../src/metrics/metric-registry'; | ||
|
||
describe('MetricProducerManager()', () => { | ||
const registry: MetricRegistry = new MetricRegistry(); | ||
const metricProducer = registry.getMetricProducer(); | ||
const registryOther: MetricRegistry = new MetricRegistry(); | ||
const metricProducerOther = registryOther.getMetricProducer(); | ||
|
||
beforeEach(() => { | ||
metricProducerManagerInstance.removeAll(); | ||
}); | ||
|
||
describe('add()', () => { | ||
it('should throw an error when the metricproducer is null', () => { | ||
assert.throws(() => { | ||
metricProducerManagerInstance.add(null); | ||
}, /^Error: Missing mandatory metricProducer parameter$/); | ||
}); | ||
|
||
it('add metricproducer', () => { | ||
metricProducerManagerInstance.add(metricProducer); | ||
const metricProducerList = | ||
metricProducerManagerInstance.getAllMetricProducer(); | ||
|
||
assert.notDeepEqual(metricProducerList, null); | ||
assert.equal(metricProducerList.size, 1); | ||
}); | ||
|
||
it('should not add same metricproducer metricProducerManagerInstance', | ||
() => { | ||
metricProducerManagerInstance.add(metricProducer); | ||
metricProducerManagerInstance.add(metricProducer); | ||
metricProducerManagerInstance.add(metricProducer); | ||
const metricProducerList = | ||
metricProducerManagerInstance.getAllMetricProducer(); | ||
|
||
assert.equal(metricProducerList.size, 1); | ||
assert.ok(metricProducerList.has(metricProducer)); | ||
}); | ||
|
||
it('should add different metricproducer metricProducerManagerInstance', | ||
() => { | ||
metricProducerManagerInstance.add(metricProducer); | ||
metricProducerManagerInstance.add(metricProducerOther); | ||
const metricProducerList = | ||
metricProducerManagerInstance.getAllMetricProducer(); | ||
|
||
assert.equal(metricProducerList.size, 2); | ||
assert.ok(metricProducerList.has(metricProducer)); | ||
assert.ok(metricProducerList.has(metricProducerOther)); | ||
}); | ||
}); | ||
|
||
describe('remove()', () => { | ||
it('should throw an error when the metricproducer is null', () => { | ||
assert.throws(() => { | ||
metricProducerManagerInstance.add(null); | ||
}, /^Error: Missing mandatory metricProducer parameter$/); | ||
}); | ||
|
||
it('remove metricproducer', () => { | ||
metricProducerManagerInstance.add(metricProducer); | ||
|
||
const metricProducerList = | ||
metricProducerManagerInstance.getAllMetricProducer(); | ||
assert.equal(metricProducerList.size, 1); | ||
assert.ok(metricProducerList.has(metricProducer)); | ||
|
||
metricProducerManagerInstance.remove(metricProducer); | ||
assert.equal(metricProducerList.size, 0); | ||
}); | ||
}); | ||
}); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on what
getMetricRegistry
does, you might be able to make these properties instead of getter methods (or even potentially replacing this class for a plain module with consts - just like the previous PR feedback).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only reason to use getter here is to make apis consistent across other language libraries. WDYT? I am happy to change as per your suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I'm not a fan of classes used only for namespacing, since Node modules already provide a namespace and classes that are the only export in a file end up causing double-namespacing for anyone using
require
instead of ES6 import syntax. That being said, I can understand the consistency argument.One thing to note, if this were a plain module end users get some extra flexibility with importing, which might be beneficial. Eg:
Importing like this which provides similar syntax to the class implementation
Or importing only what's needed to simplify code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed your question might have been specifically just about
.getMetricProducerManager()
vs.metricProducerManager
- rather than also the replacement of the class with consts 😄I don't have a strong opinion either way, since both are incredibly similar to the end user. All things equal I'd lean to not having to do a function call vs having to do one.