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

adds proxy RSocket for WeightedStatsRequestInterceptor #976

Merged
merged 2 commits into from
Feb 26, 2021

Conversation

spencergibb
Copy link
Contributor

Signed-off-by: Spencer Gibb sgibb@pivotal.io

Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

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

Sorry for the slow response.

I'm not sure if this needs to be tied to the interceptor. Perhaps something like this would make it more generally useful:

public class WeightedStatsRSocketProxy extends RSocketProxy implements WeightedStats {

	private final WeightedStats stats;


	public WeightedStatsRSocketProxy(RSocket source, WeightedStats stats) {
		super(source);
		this.stats = stats;
	}

	@Override
	public double higherQuantileLatency() {
		return this.stats.higherQuantileLatency();
	}

	@Override
	public double lowerQuantileLatency() {
		return this.stats.lowerQuantileLatency();
	}

	@Override
	public int pending() {
		return this.stats.pending();
	}

	@Override
	public double predictedLatency() {
		return this.stats.predictedLatency();
	}

	@Override
	public double weightedAvailability() {
		return this.stats.weightedAvailability();
	}
}

@spencergibb
Copy link
Contributor Author

In conversations with @OlegDokuka, the idea was that the class wouldn't be public.

@spencergibb
Copy link
Contributor Author

@rstoyanchev maybe a package protected class, but with the static creator in the interceptor still?

@rstoyanchev
Copy link
Contributor

maybe a package protected class, but with the static creator in the interceptor still?

Isn't that what's currently proposed?

How about exposing via default method on WeightedStats, something like:

default RSocket wrap(RSocket rsocket) {
    return new WeightedStatsRSocketProxy(rsocket, this); // instantiate package private class
}

Moves wrap() method to WeightedStats default method.
@spencergibb
Copy link
Contributor Author

Done @rstoyanchev

@OlegDokuka OlegDokuka changed the title Adds proxy RSocket for WeightedStatsRequestInterceptor adds proxy RSocket for WeightedStatsRequestInterceptor Feb 26, 2021
@OlegDokuka OlegDokuka merged commit cad188e into rsocket:master Feb 26, 2021
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.

3 participants