[go: nahoru, domu]

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

Make WebSocketMessageBrokerStats optionally export stats to micrometer #31604

Closed
nyilmaz opened this issue Nov 14, 2023 · 3 comments
Closed

Make WebSocketMessageBrokerStats optionally export stats to micrometer #31604

nyilmaz opened this issue Nov 14, 2023 · 3 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) theme: observability An issue related to observability and tracing type: enhancement A general enhancement
Milestone

Comments

@nyilmaz
Copy link
nyilmaz commented Nov 14, 2023

Currently we access stats via logs which WebSocketMessageBrokerStats produce. Would you consider exporting those metrics to any stats collector? If interested, we can create a pr for this.

https://stackoverflow.com/questions/57481608/how-to-access-and-export-websocketmessagebrokerstats-internal-counts

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 14, 2023
@jhoeller jhoeller added in: web Issues in web modules (web, webmvc, webflux, websocket) theme: observability An issue related to observability and tracing labels Jan 3, 2024
@bclozel
Copy link
Member
bclozel commented Jan 4, 2024

Sorry for the delayed response.

Spring Framework only depends on the Observation API and this use case doesn't fit as Observation is all about timer metrics and traces. Here, we could expose stats information and let developers bind Gauges to their MeterRegistry using the "micrometer-core" dependency.

The log line contains several parts:

WebSocketSession[...], stompSubProtocol[...], stompBrokerRelay[...], inboundChannel[...], outboundChannel[...], sockJsScheduler[...]

The "inboundChannel", "outboundChannel" and "sockJsScheduler" are all Stringly typed as this information is extracted directly from ThreadPoolExecutor#toString. I don't think we can expose more detailed stats. At best, we could expose the Executor directly and let developers extract information from there.

As for the "WebSocketSession", "stompSubProtocol", "stompBrokerRelay", they all have dedicated Stats interface that are already public. We could add methods like the following to WebSocketMessageBrokerStats and deprecate the String versions to avoid redundancy:

@Nullable
public StompSubProtocolHandler.Stats getStompSubProtocolStats() {
	return (this.stompSubProtocolHandler != null ? this.stompSubProtocolHandler.getStats() : null);
}

This would allow applications to register gauges with the metric names and tags of their choices:

MeterRegistry meterRegistry = ...;

Gauge.builder("spring.stomp.frames", stats.getStompSubProtocolStats(), StompSubProtocolHandler.Stats::getTotalConnect)
			.tag("type", "CONNECT")
			.description("number of CONNECT frames processed")
			.register(meterRegistry);

Gauge.builder("spring.stomp.frames", stats.getStompSubProtocolStats(), StompSubProtocolHandler.Stats::getTotalConnected)
			.tag("type", "CONNECTED")
			.description("number of CONNECTED frames processed")
			.register(meterRegistry);

The only downside here is that such metrics would probably fit more with Counter, but counters need to be incremented manually by the instrumentation rather than polling like Gauges do.

@nyilmaz What do you think about this proposal?

@bclozel bclozel added the status: waiting-for-feedback We need additional information before we can continue label Jan 4, 2024
@nyilmaz
Copy link
Author
nyilmaz commented Jan 10, 2024

This would be perfectly OK and enough form most of the time, also leaving metering library free to choose. Thanks for your attention 👍 and observation! A separate bean can be defined to automatically wired in to collect those metrics and expose them via prometheus, however my last sentence addresses spring-boot's side I think.

How can we proceed from now on?

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jan 10, 2024
@bclozel bclozel added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Jan 10, 2024
@bclozel bclozel self-assigned this Jan 10, 2024
@bclozel bclozel modified the milestones: 6.1.x, 6.2.x Jan 10, 2024
@bclozel
Copy link
Member
bclozel commented Jan 10, 2024

I'll take care of it. Thanks for your feedback!

@bclozel bclozel modified the milestones: 6.2.x, 6.2.0-M4 May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) theme: observability An issue related to observability and tracing type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants