Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(fabric): Multiple objects of the ledger connector class throws errors with prometheus exporter object #634

Closed
jagpreetsinghsasan opened this issue Mar 5, 2021 · 1 comment · Fixed by #652
Assignees
Labels
bug Something isn't working

Comments

@jagpreetsinghsasan
Copy link
Contributor

Describe the bug
When multiple objects of the ledger-connector-fabric are instantiated, it causes issues with prometheus exporter object initialization.

To Reproduce

Instantiate multiple ledger-connector-fabric objects.
prometheus exporter object throws error stating
Error: A metric with the name process_cpu_user_seconds_total has already been registered.

Expected behavior
Multiple object initialization should occur without any issues.

Hyperledger Cactus Plugins/Connectors Used

  • Fabric

Additional context

Mostly the issue is with the collectDefaultMetrics function located under /cactus/packages/cactus-plugin-ledger-connector-fabric/src/main/typescript/prometheus-exporter/prometheus-exporter.ts. Requires more root cause analysis before concluding the reason of the error.

@jagpreetsinghsasan jagpreetsinghsasan added the bug Something isn't working label Mar 5, 2021
@petermetz
Copy link
Contributor

@jagpreetsinghsasan My recommendation here is to pull the exporter state out of the individual plugins to a dedicated singleton-ish** instance and then have the N number of Fabric connector plugins share that instance among themselves.

Easiest would be to have a PrometheusExporterFabricFactory (or similarly named) factory class that has a static getOrCreate() method that internally checks if there has been any instantiation of the Prometheus exporter and then either creates a new instance of returns the existing one to which the reference can be stored on the global object under a hardcoded uuid as the property key so as to make conflicts practically impossible but also guarantee that different imports of the same factory class will not cause a split brain scenario with multiple singletons even in the same NodeJS process.

There's other options but if you ask me they are all much more heavy handed rabbit-holes, most notably IoC containers, that will still mostly suffer from the same problems anyway due to some specifics of the language/NodeJS itself.

** the ish suffix is there because of the various fine prints when it comes to singletons such as that singleton only in the context of a single NodeJS process, not so much if one uses NodeJS clustering for example.

jagpreetsinghsasan added a commit to jagpreetsinghsasan/cactus that referenced this issue Mar 10, 2021
	Primary Change
	--------------

	1. Adding the default_metrics to the registry object which will differ for different objects.

	Fixes hyperledger-cacti#634

Signed-off-by: Jagpreet Singh Sasan <[email protected]>
kikoncuo pushed a commit that referenced this issue Mar 11, 2021
	Primary Change
	--------------

	1. Adding the default_metrics to the registry object which will differ for different objects.

	Fixes #634

Signed-off-by: Jagpreet Singh Sasan <[email protected]>
AzaharaC pushed a commit to kikoncuo/cactus that referenced this issue Mar 12, 2021
	Primary Change
	--------------

	1. Adding the default_metrics to the registry object which will differ for different objects.

	Fixes hyperledger-cacti#634

Signed-off-by: Jagpreet Singh Sasan <[email protected]>
jordigiam pushed a commit to kikoncuo/cactus that referenced this issue Apr 8, 2021
	Primary Change
	--------------

	1. Adding the default_metrics to the registry object which will differ for different objects.

	Fixes hyperledger-cacti#634

Signed-off-by: Jagpreet Singh Sasan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants