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

Endpoint availability is not exposed to the caller code #1974

Closed
ivshch opened this issue Jul 29, 2020 · 9 comments
Closed

Endpoint availability is not exposed to the caller code #1974

ivshch opened this issue Jul 29, 2020 · 9 comments

Comments

@ivshch
Copy link

ivshch commented Jul 29, 2020

  • List of NuGet packages and version that you are using: Microsoft.ApplicationInsights (2.10.0), also checked on 2.14.0.
  • Runtime version: netstandard2.0
  • Hosting environment: Azure Web App, also chacked on Windows 10, Windows Server 2016.

Bug description

Our current task requires to write the human-readable file logs only at the moments when the AppInsights endpoint is unreachable.

This is quite close to what ServerTelemetryChannel does, but without gzipping and using the rolling mechanism. Our current default configuration (large cloud-based project) does not set the StorageFolder parameter for the ServerTelemetryChannel, so the channel does not enable its intermediate disk storage capabilities. Even if we set the StorageFolder it writes the compressed files that are not suitable for the support staff. Our goal is to make a solution that would allow them to investigate the connectivity problems after (for example) a week of AppInsights silence.

Most problem is we don't have an indicator of whether the AppInsights is assuming the connection loss, or assuming the connection is working.

So, investigating the AppInsights sources gives us the two classes that are monitoring the network status:

NetworkAvailabilityTransmissionPolicy, and
BackoffLogicManager

Both classes are instantiated quite deep in the internals of the ServerTelemetryChannel class, access to its properties is not possible from the calling code.

Please fix the implementation to allow access to the current endpoint availability state.

To Reproduce

On any working AppInsights project, at the runtime set the internet connection to the disabled state. While the endpoint is unavailable, there are no exposed properties/events/hooks to know that AppInsights is not sending the data.

@ivshch ivshch added the bug label Jul 29, 2020
@cijothomas
Copy link
Contributor

#1887 would solve this issue.

@ivshch
Copy link
Author

ivshch commented Jul 29, 2020

Thank you, looks very promising.
Looking forward to seeing the milestone assigned to #1887.

@cijothomas
Copy link
Contributor

Thank you, looks very promising.
Looking forward to seeing the milestone assigned to #1887.

It is expected to be part of next beta and stable.

@ivshch
Copy link
Author

ivshch commented Jul 29, 2020

Thank you.
I was looking through the code and got a question:
what if the request is failed with the HttpRequestException (in case of general network failure), it seems that the delegate is not invoked?
This probably suits the initial idea of gathering metric of successful requests, but if I'm correct it makes it unusable in our scenario - detecting the connection loss.

Could it improve your solution if the delegate allows you to collect failures also?

@cijothomas
Copy link
Contributor

Thank you.
I was looking through the code and got a question:
what if the request is failed with the HttpRequestException (in case of general network failure), it seems that the delegate is not invoked?
This probably suits the initial idea of gathering metric of successful requests, but if I'm correct it makes it unusable in our scenario - detecting the connection loss.

Could it improve your solution if the delegate allows you to collect failures also?

The PR is not final code. Feel free to add comments in the PR.

@rajkumar-rangaraj
Copy link
Member

@ivshch PR has modified code. It should cover your scenario. Could you please check?

@ivshch
Copy link
Author

ivshch commented Jul 30, 2020

Thank you, I am analyzing the change.

Am I right assuming the HttpRequestException would be propagated further without being handled in Transmission?
Is there a way to get some indication that the HttpRequestException occurred?
I mean the code instantiating the TelemetryChannel.

@ivshch
Copy link
Author

ivshch commented Aug 7, 2020

@ivshch PR has modified code. It should cover your scenario. Could you please check?

I have checked the modified code. This implementation does not allow us to proceed if the HttpRequestException has occurred.
Maybe you can suggest a workaround for catching it?

@github-actions
Copy link

This issue is stale because it has been open 300 days with no activity. Remove stale label or comment or this will be closed in 7 days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants