Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

[PIE-1580] Metrics for smart contract permissioning actions #1492

Merged

Conversation

Errorific
Copy link
Contributor

plumbing some metrics into the permissioning controllers so we can measure their performance.

Copy link
Contributor

@usmansaleem usmansaleem left a comment

Choose a reason for hiding this comment

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

👍 Looks good.

this.contractAddress = contractAddress;
this.transactionSimulator = transactionSimulator;

this.checkCounter =
Copy link
Contributor

Choose a reason for hiding this comment

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

If we have the counter for permitted and unpermitted, do we really need the total counter or can we just infer from the other two metrics?
I'm not aware of the impact of the metrics in system resources so maybe we are better playing safe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's an outcome where the permissioning check crashes (no contract, bad transaction etc) which increments the total counter but not the other 2. I'm assuming the metrics are no heavier than adding log lines so I decided to be distinct about all the things we might be checking.

this.contractAddress = contractAddress;
this.transactionSimulator = transactionSimulator;

this.checkCounter =
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing about the total checks counter.

@lucassaldanha
Copy link
Contributor

I'm not sure how the names of the metrics are put together later, but I wonder if we could create some hierarchical namespace to make it easier to understand.

e.g. we could have a "parent" counter for node permissioning and one for account permissioning. In the case of node permissioning, we could then have the metrics for each provider, and for account permissioning the metrics for the local config and onchain checks.

Something like:

permissioning.node.permitted = total of permitted communication in the NodePermissioningController
permissioning.node.unpermitted = total of permitted communication in the NodePermissioningController
permissioning.node.local_config.permitted = total of permitted communication in the NodeLocalConfigPermissioningProvider
permissioning.node.smart_contract.permitted = total of permitted communication in the NodeSmartContractPermissioningProvider
...

@Errorific Errorific merged commit b5ef926 into PegaSysEng:master May 26, 2019
@Errorific Errorific deleted the feature/PIE-1580.Permissioning_metrics branch May 26, 2019 21:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants