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

Always add proxy.address tag for the client metrics #3230

Merged
merged 2 commits into from May 7, 2024
Merged

Conversation

violetagg
Copy link
Member

When there is no proxy add na as a value tag.

Fixes #3218

When there is no proxy add `na` as a value tag.

Fixes #3218
@violetagg violetagg added type/bug A general bug warn/regression A regression from a previous release labels May 7, 2024
@violetagg violetagg added this to the 1.1.19 milestone May 7, 2024
@violetagg violetagg requested a review from a team May 7, 2024 12:09
@pderop
Copy link
Member

pderop commented May 7, 2024

in MicrometerChannelMetricsRecorder.java, line 176, is the following method still used ? if not, can we remove it ?

	@Nullable
	public final Timer getTlsHandshakeTimer(String name, @Nullable String address, String status) {
		MeterKey meterKey = new MeterKey(null, address, null, null, status);
		return MapUtils.computeIfAbsent(tlsHandshakeTimeCache, meterKey,
				key -> filter(Timer.builder(name)
				                   .tags(REMOTE_ADDRESS, address, STATUS, status)
				                   .register(REGISTRY)));
	}

@violetagg
Copy link
Member Author

in MicrometerChannelMetricsRecorder.java, line 176, is the following method still used ? if not, can we remove it ?

	@Nullable
	public final Timer getTlsHandshakeTimer(String name, @Nullable String address, String status) {
		MeterKey meterKey = new MeterKey(null, address, null, null, status);
		return MapUtils.computeIfAbsent(tlsHandshakeTimeCache, meterKey,
				key -> filter(Timer.builder(name)
				                   .tags(REMOTE_ADDRESS, address, STATUS, status)
				                   .register(REGISTRY)));
	}

It is not used, unfortunately the class/method is public and we cannot remove it. May be just deprecate it as unused?

@pderop
Copy link
Member

pderop commented May 7, 2024

yes, indeed, it can't be removed, in this case, we can deprecate it.

@violetagg
Copy link
Member Author

please check the new commit

@violetagg
Copy link
Member Author

@pderop Thanks for the review!

@violetagg violetagg merged commit 4fdac19 into 1.1.x May 7, 2024
11 of 14 checks passed
violetagg added a commit that referenced this pull request May 7, 2024
@violetagg violetagg deleted the issue-3218 branch May 7, 2024 13:45
violetagg added a commit that referenced this pull request May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug A general bug warn/regression A regression from a previous release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Micrometer Prometheus metrics export fails due to conditional proxy.address tag
2 participants