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

Add upload, download and append stats #553

Closed
wants to merge 6 commits into from
Closed

Add upload, download and append stats #553

wants to merge 6 commits into from

Conversation

HDegroote
Copy link
Contributor

@HDegroote HDegroote commented Aug 21, 2024

The way of passing the stats object to all sessions is pretty ad-hoc--if there's a cleaner, more explicit way, that would be preferable,

stats now live in Core instead

index.js Outdated Show resolved Hide resolved
@HDegroote HDegroote marked this pull request as draft August 21, 2024 11:45
@HDegroote HDegroote marked this pull request as ready for review August 21, 2024 11:50
@HDegroote HDegroote marked this pull request as draft August 22, 2024 11:32
@HDegroote
Copy link
Contributor Author

Marking this as draft--it needs an iteration on where these stats should live exactly

ready () {
return this.opening
}

_onupload (index, value, from) {
const byteLength = value.byteLength - this.padding

this.core.stats.blocksUploaded++
this.core.stats.bytesUploaded += byteLength
Copy link
Contributor

Choose a reason for hiding this comment

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

should not live in the sesison, move to replicator or who ever is triggering this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is emitted from the replicator, but the replicator has no direct access to the byteLength as calculated here, so I need to look into another way to calculate that (from what I can tell, it has no access to info about the encryption and its padding, but it can probably know the byte length of the block by checking Hypercore.core, which it does have access to).

Copy link
Contributor

Choose a reason for hiding this comment

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

byteLength is value.byteLength, this is actualy wrong from a stats pov

Copy link
Contributor

Choose a reason for hiding this comment

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

the padding is due to the session being encrypted, nothing to do with the size of the block it pulled from the network

@@ -711,6 +727,9 @@ module.exports = class Hypercore extends EventEmitter {
if (value) {
const byteLength = value.byteLength - this.padding

this.core.stats.blocksDownloaded++
this.core.stats.bytesDownloaded += byteLength
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

}

this.core.stats.blocksAppended += blocks.length
this.core.stats.bytesAppended += totalByteLength
Copy link
Contributor

Choose a reason for hiding this comment

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

move to core.append instead of the session

@HDegroote
Copy link
Contributor Author

Closing, since this is done more cleanly in
#553
#557

@HDegroote HDegroote closed this Sep 3, 2024
@HDegroote HDegroote deleted the add-stats branch September 3, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants