8000 Adding NoopScope, TestScope and Sanitizer by ravirajj · Pull Request #43 · uber-java/tally · GitHub
[go: up one dir, main page]
Skip to content

Adding NoopScope, TestScope and Sanitizer #43

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.

Sign up for GitHub

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

Jump to bottom
Open
ravirajj wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Adding NoopScop 8000 e, TestScope and Sanitizer #43

ravirajj wants to merge 5 commits into from

Conversation

@ravirajj
Copy link
@ravirajj ravirajj commented Sep 7, 2019 •
edited
Loading

This pull request contains the following commits to bring uber-java/tally on par with uber-go/tally:

  • Upgrade to Java 8 (previous attempt Update source&target language level to java8 #39)
    • enable the use of static interface methods to hide implementation classes.
    • remove synchronized statements and enable the use of ConcurrentMap.putIfAbsent.
  • Only exposing public interfaces, implementations are package-private:
    • Moving BucketPairImpl.bucketPairs() to BucketPairs.create().
  • Adding NoopScope (previous attempt Noop scope #40), NullStatsReporter
  • Adding TestScope interface and factory methods
  • Adding Sanitizer interface and implementation

@JigarJoshi
Copy link
Contributor
JigarJoshi commented Sep 9, 2019

Repeats the features from

#39
#40

@andrewmains12
Copy link
Collaborator
andrewmains12 commented Sep 9, 2019

Only exposing public interfaces, implementations are package-private.

This is unfortunately probably backwards incompatible, and I'm not sure worth a new version. Should we back that out? cc @prateek

@mway mway requested review from justinjc and prateek September 9, 2019 18:18
@ravirajj
Copy link
Author
ravirajj commented Sep 9, 2019

Only exposing public interfaces, implementations are package-private.

This is unfortunately probably backwards incompatible, and I'm not sure worth a new version. Should we back that out? cc @prateek

The main change here is moving BucketPairImpl. bucketPairs() to BucketPairs.create(), since Java 8 allows you to add static methods to interfaces. Alternatively, we can add BucketPairFactory.createbucketPairs() if we are using Java 7 and hide the *Impl classes.

@ravirajj
Copy link
Author
ravirajj commented Sep 9, 2019 •
edited
Loading

Repeats the features from
#39
Plus other changes to make Travis pass!
#40
Different Implementation. This implementation uses NullStatsReporter just like uber-go/tally: https://github.com/uber-go/tally/blob/master/scope.go#L32

@zmanji zmanji mentioned this pull request Dec 18, 2023
Merged
SokolAndrey pushed a commit that referenced this pull request Dec 18, 2023
@zmanji
Add TestScope (#120)
774a8c1 
This commit plucks select changes from #69 and #43
to bring uber-java/tally on par with uber-go/tally.

It adds a `TestScope` class which allows users of the library to get a
`Snapshot` of all the metrics emitted by the scope. Users can then assert if
certain metrics were created.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@prateek prateek Awaiting requested review from prateek

@justinjc justinjc Awaiting requested review from justinjc

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ravirajj @JigarJoshi @andrewmains12
0