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

Retain summary statistics when serializing/deserializing #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hassy
Copy link

@hassy hassy commented May 21, 2021

The changes in this PR fix toProto/fromProto to retain summary statistics (i.e. min, max and sum).

Thank you for a great library @brimtown!

@brimtown brimtown self-requested a review May 24, 2021 17:21
@brimtown
Copy link
Collaborator

Hey @hassy ! Thank you for the PR, and glad it's finding some use in Artillery!

One thing that's not immediately obvious from this repo is that the protobuf definition is shared across our other language implementations of DDSketch (Java, Go). We will need to take those into account in thinking about how to land this, and plan to update those accordingly.

cc @CharlesMasson

@hassy
Copy link
Author

hassy commented May 26, 2021

That makes sense! Let me know if there's anything I can do to help land this change. We'll pin the dependency to our own fork for now for Artillery, but of course I'd love for this to be in the official distribution as soon as possible. :)

hassy added a commit to artilleryio/artillery that referenced this pull request Jul 29, 2021
Use our own fork of sketches-js until the PR is merged upstream:
DataDog/sketches-js#13
hassy added a commit to artilleryio/artillery that referenced this pull request Jul 29, 2021
Use our own fork of sketches-js until the PR is merged upstream:
DataDog/sketches-js#13
hassy added a commit to artilleryio/artillery that referenced this pull request Jul 29, 2021
Use our own fork of sketches-js until the PR is merged upstream:
DataDog/sketches-js#13
hassy added a commit to artilleryio/artillery that referenced this pull request Jul 29, 2021
Use our own fork of sketches-js until the PR is merged upstream:
DataDog/sketches-js#13
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