search

ethersphere / swarm

Swarm: Censorship resistant storage and communication infrastructure for a truly sovereign digital society
https://swarm.ethereum.org/
GNU Lesser General Public License v3.0
488 stars 110 forks source link

Harmonize `AccountingApi` with actual implementation #1591

Open holisticode opened 5 years ago

holisticode commented 5 years ago

AccountingApi has been implemented in an early iteration of Swap.

Some of its concepts are outdated (balance calculation) and thus this needs to be refactored

mortelli commented 5 years ago

Also: look into possibly merging this API with the swap API.

holisticode commented 4 years ago

So the AccountingAPI is actually the API to access the metrics defined in p2p/protocols. It is thus NOT an API to access actual balances with nodes and peers, but to access metrics.

The metrics system is optional. The metrics system is most useful if it is persisted, and the current configuration for metrics is to write to an influxdb, where then the metrics could be aggregated over sessions and queried.

The AccountingAPI would currently access non-aggregated metrics by querying directly the counters of the node, which per se are not persisted.

It is therefore of limited use. My suggestion is to get rid of this API and to access the actual metrics via persisted influxdb and visualize them via grafana, as we do ourselves for development.

janos commented 4 years ago

Have in mind that metrics is the instrumentation and infrastructure tool and that it is developed in that purpose in mind. I know that accounting uses metrics from start but I think that it creates a coupling between two very different aspects of the application.

holisticode commented 4 years ago

No, accounting does not use metrics from start. Metrics are metrics and is indeed used for instrumentation only. Accounting uses its own datastore (LevelDB) for storing balances and cheques and doesn't use the metrics system for that.

janos commented 4 years ago

I see, I am sorry, I must have confused it with something else that I've saw earlier where metrics are used where a counter could be used instead. I have got impression that that was related to AccountingRegistry, but only some tests are in swap and p2p/protocols, where the correctness of accounting is validated by the instrumentation tools.

holisticode commented 4 years ago

Yes, indeed, tests do - currently - rely on instrumentation for proper synchronization, because that is the most easy way to do it.

Of course, this also could be understood as not optimal, as it couples tests with metrics / instrumentation, in which case the equivalent data points offered by the metrics used should be made available by other means.