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

[Service Bus] Detect connection status #8561

Closed
giggio opened this issue Nov 1, 2019 · 70 comments
Closed

[Service Bus] Detect connection status #8561

giggio opened this issue Nov 1, 2019 · 70 comments
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. Service Attention Workflow: This issue is responsible by Azure service team. Service Bus
Milestone

Comments

@giggio
Copy link

giggio commented Nov 1, 2019

Right now it does not seem possible to detect the connection state with Azure Service Bus. There is no property that indicates if we are connected or if there is some connectivity issue (or other issue, like subscription deleted etc).

Describe the solution you'd like
I want a boolean property on the SubscriptionClient that tells me if it is connected. And I want an event on connection changes, i.e. Connected and Disconnected.

Describe alternatives you've considered
The alternative I'm using now uses Reflection, which is fragile. It does a little bit like this:

var subscriptionClient = new SubscriptionClient(connStr, topic, subscription, ReceiveMode.PeekLock);
var conn = subscriptionClient.ServiceBusConnection;
FaultTolerantAmqpObject<AmqpConnection> connectionManager = (FaultTolerantAmqpObject<AmqpConnection>)typeof(ServiceBusConnection).GetProperty("ConnectionManager", BindingFlags.Instance | BindingFlags.NonPublic).GetMethod.Invoke(conn, null);
var amqpConnection = await connectionManager.GetOrCreateAsync(TimeSpan.FromSeconds(2));
var amqpSessionSettings = new AmqpSessionSettings { Properties = new Fields() };
while (true)
{
    var session = amqpConnection.CreateSession(amqpSessionSettings);
    var connected = false;
    try
    {
        await session.OpenAsync(TimeSpan.FromSeconds(2));
        await session.CloseAsync(TimeSpan.FromSeconds(2));
        connected = true;
    }
    catch (TimeoutException)
    {
    }
    Console.WriteLine(connected);
    Thread.Sleep(2000);
}

Additional context
I'll use this to inform the healthcheck of a container on a Kubernetes environment.

We shouldn't need to have to use reflection. And I get that the library is resilient and will reconnect. But I need to setup scaling, alerts and other infrastructure based on the fact that there is a worker doing what it is supposed to be doing. If for some reason a container gets disconnected from Service Bus, I need to know, I might setup infrastructure that will recreate it, setup alarms, etc. My queues/topics might be filling up, and I don't get to react to it.

@triage-new-issues triage-new-issues bot added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Nov 1, 2019
@AlexGhiondea AlexGhiondea added Client This issue points to a problem in the data-plane of the library. Service Attention Workflow: This issue is responsible by Azure service team. Service Bus labels Nov 1, 2019
@triage-new-issues triage-new-issues bot removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Nov 1, 2019
@ghost
Copy link

ghost commented Nov 1, 2019

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @jfggdl

1 similar comment
@ghost
Copy link

ghost commented Nov 1, 2019

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @jfggdl

@AlexGhiondea
Copy link
Contributor

/cc @binzywu.

Also, @jsquire to see how this feedback applies to our Track 2 libraries.

@jfggdl jfggdl added this to the Sprint 164 milestone Nov 1, 2019
@jfggdl
Copy link

jfggdl commented Nov 1, 2019

@giggio, thanks for opening this issue.
@axisc or @makam: Is there a way we can expose the connection status from the service to the SDKs, if that is not currently the case?

@axisc
Copy link

axisc commented Nov 11, 2019

We'll investigate this and add to the backlog.

@SeanFeldman
Copy link
Contributor

I want a boolean property on the SubscriptionClient that tells me if it is connected.

That property exists on all clients, including SubscriptionClient - IsClosedOrClosing.

@giggio
Copy link
Author

giggio commented Nov 15, 2019

That property exists on all clients, including SubscriptionClient - IsClosedOrClosing.

If we pull the network cable and put the computer on airplane mode, so cutting all network communication, that property still shows as false. So we cannot rely on it.

@SeanFeldman
Copy link
Contributor

I did not claim you can rely on it, just pointed out there there is a property that is supposed to indicate what's the state of the connectivity. The fact that it doesn't work as it supposed to is a bug that should be looked into (@axisc might already know about it).

For the scenario you've described, you really need not a property, but an event. Which you've also mentioned. That would be a feature as today it doesn't exist AFAIK.

@tomkerkhove
Copy link
Member

This is a great feature request - We'd like to use this as well so that we can use it as part of our health probe approach.

THis is important as Kubernetes requires this to kill dead pods and recreate them.

@giggio
Copy link
Author

giggio commented Dec 5, 2019

@tomkerkhove This is exactly what I need to do and the reason I opened this issue.

@tomkerkhove
Copy link
Member

Any feedback/timeline on this one @axisc?

@tomkerkhove
Copy link
Member

While we're asking, would be good if there's a built-in .NET Core Health Check that works with this feature

@giggio
Copy link
Author

giggio commented Dec 17, 2019

Everyone interested in this issue, please add your 👍 up there so they can prioritize it.

@jsquire jsquire modified the milestones: Sprint 164, Backlog Feb 26, 2020
@jsquire jsquire added the issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. label Apr 15, 2022
@ghost
Copy link

ghost commented Apr 15, 2022

Hi @giggio. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text “/unresolve” to remove the “issue-addressed” label and continue the conversation.

@jsquire jsquire closed this as completed Apr 15, 2022
@SeanFeldman
Copy link
Contributor

@jsquire or @JoshLove-msft, is this API documented anywhere?

@jsquire
Copy link
Member

jsquire commented Apr 15, 2022

@SeanFeldman: Documentation can currently be found on MS Docs:

I'll defer to @JoshLove-msft for any additional sources and future plans.

@SeanFeldman
Copy link
Contributor

Based on what I’ve seen in the past, XML docs don’t provide as much help to the teams that don’t use ASB extensively as elaborate documentation. Just in the links above, I can see that two steps are required to get the metrics.

Adding it to Diagnostics section could be a good start.

@sohanlal311
Copy link

@JoshLove-msft Could you please point me out to the Java API for the same?

@sohanlal311
Copy link

/unresolve

@ghost
Copy link

ghost commented Apr 19, 2022

Hi sohanlal311, only the original author of the issue can ask that it be unresolved. Please open a new issue with your scenario and details if you would like to discuss this topic with the team.

@sohanlal311
Copy link

@JoshLove-msft Could you please point me out to the Java API for the same?

@JoshLove-msft
Copy link
Member

Hi @sohanlal311 this feature is not yet available in the Java library.
/cc @ki1729

@sohanlal311
Copy link

@JoshLove-msft Could you please let me know if we need to open another issue for Java version?

@JoshLove-msft
Copy link
Member

@JoshLove-msft Could you please let me know if we need to open another issue for Java version?

@ki1729 do you know if there is already a Java version of this issue file?

@JoshLove-msft
Copy link
Member

Reopening so that we can collect feedback on the beta feature before GA.

@ghost
Copy link

ghost commented May 2, 2022

Hi @giggio, since you haven’t asked that we “/unresolve” the issue, we’ll close this out. If you believe further discussion is needed, please add a comment “/unresolve” to reopen the issue.

@ghost ghost closed this as completed May 2, 2022
@sohanlal311
Copy link

@ki1729 Could you please let me know if there is already a Java version of this issue?

@sohanlal311
Copy link

@ki1729 Could you please provide an update on this one?

@sohanlal311
Copy link

@JoshLove-msft Could you please let me know if there is already a Java version of this issue?

@stliu
Copy link
Member

stliu commented Aug 16, 2022

@JoshLove-msft @joshfree @shankarsama seems this is not resolved yet, shall we reopen it? we have #21976 depends on it as well

@joshfree
Copy link
Member

@stliu let's open a new issue in https://github.com/azure/azure-sdk-for-java/ repo for the Java SDK specific ask, and we can link that back to here; rather than adding Java noise to the .NET repo. :)

@stliu
Copy link
Member

stliu commented Aug 17, 2022

ah, my mistake didn't check the repo :)

@ki1729
Copy link

ki1729 commented Aug 17, 2022

@stliu I have created a new issue in the Java repo to track this feature request : Azure/azure-sdk-for-java#30507

@ki1729
Copy link

ki1729 commented Aug 17, 2022

@sohanlal311 : The corresponding issue for Java is here: Azure/azure-sdk-for-java#30507

@scgbear
Copy link
Member

scgbear commented Sep 27, 2022

@jsquire or @JoshLove-msft, there an ETA on when ServiceBusTransportMetrics will make it's way out of the https://www.nuget.org/packages/Azure.Messaging.ServiceBus/7.8.0-beta.2 package ?

All of the links to documentation you provided above resolve to a 404.

This feature die on the vine?

Are there alternative approaches we should be taking to solve the problem of determining connection status?

@JoshLove-msft
Copy link
Member

@scgbear, yes we are planning on taking a different approach here. @lmolkova do you have any timelines for when the metrics work will begin for .NET?

@bar10dr
Copy link

bar10dr commented Jan 24, 2023

How on earth does it take more than three years to implement two simple events that informs of a connection state change?

This is such a low level basic feature.

Is the plan to bind this functionality into some sort of premium tier or additional paid service?

@github-actions github-actions bot locked and limited conversation to collaborators Apr 24, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. issue-addressed Workflow: The Azure SDK team believes it to be addressed and ready to close. Service Attention Workflow: This issue is responsible by Azure service team. Service Bus
Projects
None yet
Development

No branches or pull requests