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 a TotalThroughputSampler. #185

Merged
merged 2 commits into from
Oct 28, 2020

Conversation

magnusstahre
Copy link
Contributor

No description provided.

@magnusstahre magnusstahre requested a review from a team October 21, 2020 13:02
@maplebed
Copy link
Contributor

This looks great. Thank you for bringing it in!

Copy link
Member

@martin308 martin308 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, thanks @magnusstahre, once we get #186 in then we can refactor this to use the new shared key generation, and once #188 is merged the build should pass!

sample/totalthroughput.go Outdated Show resolved Hide resolved
@martin308
Copy link
Member

@magnusstahre I'm also happy to make the above changes to your PR if you like?

@magnusstahre magnusstahre requested a review from a team October 23, 2020 00:42
martin308 added a commit that referenced this pull request Oct 23, 2020
These builds don't have the required ENV variables to run this step. This can be seen in #185
Copy link
Member

@martin308 martin308 left a comment

Choose a reason for hiding this comment

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

One more thing I missed, sorry! Copied over some of the validation from the other dynamic samples that look like they matched up

config/sampler_config.go Outdated Show resolved Hide resolved
config/sampler_config.go Outdated Show resolved Hide resolved
Co-authored-by: Martin Holman <me@martinholman.co.nz>
@martin308 martin308 merged commit 96843de into honeycombio:main Oct 28, 2020
@magnusstahre magnusstahre deleted the totalthroughput branch October 29, 2020 18:10
@magnusstahre
Copy link
Contributor Author

@martin308 are you able to cut a release that has this change in it? thanks!

@MikeGoldsmith
Copy link
Contributor

Hey @magnusstahre - I've just published v0.13.0 👍

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.

4 participants