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

feat(iot-dev): Allow users to opt-out of treating routine disconnects as errors #1740

Merged
merged 5 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ public final class ClientConfiguration

private boolean useIdentifiableThreadNames = true;

private boolean logRoutineDisconnectsAsErrors = true;
timtay-microsoft marked this conversation as resolved.
Show resolved Hide resolved

private IotHubAuthenticationProvider authenticationProvider;

/**
Expand Down Expand Up @@ -223,6 +225,7 @@ private void setClientOptionValues(ClientOptions clientOptions)
this.threadNamePrefix = clientOptions != null ? clientOptions.getThreadNamePrefix() : null;
this.threadNameSuffix = clientOptions != null ? clientOptions.getThreadNameSuffix() : null;
this.useIdentifiableThreadNames = clientOptions == null || clientOptions.isUsingIdentifiableThreadNames();
this.logRoutineDisconnectsAsErrors = clientOptions == null || clientOptions.isLoggingRoutineDisconnectsAsErrors();

if (proxySettings != null)
{
Expand Down Expand Up @@ -646,6 +649,12 @@ public boolean isUsingIdentifiableThreadNames()
return this.useIdentifiableThreadNames;
}

public boolean isLoggingRoutineDisconnectsAsErrors()
{
// Using a manually written method here to override the name that Lombok would have given it
return this.logRoutineDisconnectsAsErrors;
}

/**
* Sets the device operation timeout
* @param timeout the amount of time, in milliseconds, that a given device operation can last before expiring
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,29 @@ public final class ClientOptions
@Builder.Default
private final boolean useIdentifiableThreadNames = true;

/**
* This option allows for routine disconnect events (such as expired SAS token disconnects
* when connecting over MQTT or MQTT_WS) to be logged at debug levels as opposed to error or
* warn levels. By default, these routine disconnects are logged at error or warn levels.
*
* Note that there is a degree of speculation involved when this client labels a disconnect
* as a routine disconnect. Generally, if the client is disconnected when the previous SAS
* token was expired, it will assume it was a routine disconnect. However, there may be
* legitimate disconnects due to network errors that could be mislabeled and not logged
* at a warning or error level if they occur around the time that a routine error is expected.
*/
@Builder.Default
private final boolean logRoutineDisconnectsAsErrors = true;

public boolean isUsingIdentifiableThreadNames()
{
// Using a manually written method here to override the name that Lombok would have given it
return this.useIdentifiableThreadNames;
}

public boolean isLoggingRoutineDisconnectsAsErrors()
{
// Using a manually written method here to override the name that Lombok would have given it
return this.logRoutineDisconnectsAsErrors;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public class IotHubTransport implements IotHubListener
private IotHubConnectionStatusChangeCallback multiplexingStateCallback;
private Object multiplexingStateCallbackContext;

private RetryPolicy multiplexingRetryPolicy = new ExponentialBackoffWithJitter();
private RetryPolicy multiplexingRetryPolicy;

// Callback for notifying the DeviceIO layer of connection status change events. The deviceIO layer
// should stop spawning send/receive threads when this layer is disconnected or disconnected retrying
Expand Down Expand Up @@ -179,6 +179,7 @@ public IotHubTransport(
this.connectionStatus = IotHubConnectionStatus.DISCONNECTED;
this.deviceIOConnectionStatusChangeCallback = deviceIOConnectionStatusChangeCallback;
this.isMultiplexing = true;
this.multiplexingRetryPolicy = new ExponentialBackoffWithJitter();
this.keepAliveInterval = keepAliveInterval;
this.sendInterval = sendInterval;
this.useIdentifiableThreadNames = useIdentifiableThreadNames;
Expand Down Expand Up @@ -1643,7 +1644,18 @@ private void updateStatus(IotHubConnectionStatus newConnectionStatus, IotHubConn
{
if (throwable == null)
{
log.debug("Updating transport status to new status {} with reason {}", newConnectionStatus, reason);
log.info("Updating transport status to new status {} with reason {}", newConnectionStatus, reason);
}
else if (this.getDefaultConfig() != null
&& !this.getDefaultConfig().isLoggingRoutineDisconnectsAsErrors()
&& newConnectionStatus == IotHubConnectionStatus.DISCONNECTED_RETRYING
&& reason == IotHubConnectionStatusChangeReason.EXPIRED_SAS_TOKEN
&& (protocol == IotHubClientProtocol.MQTT || protocol == IotHubClientProtocol.MQTT_WS))
{
// This is a special case where the user has opted out of treating the routine
// MQTT/MQTT_WS SAS token disconnects as an error for logging purposes. As such,
// log at the debug level instead of warn or error level.
log.info("Updating transport status to new status {} with reason {}", newConnectionStatus, reason);
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,25 +327,24 @@ public IotHubTransportMessage receive()
@Override
public void connectionLost(Throwable throwable)
{
log.warn("Mqtt connection lost", throwable);
timtay-microsoft marked this conversation as resolved.
Show resolved Hide resolved

this.disconnect();

if (this.listener != null)
TransportException transportException;
if (throwable instanceof MqttException)
{
TransportException transportException;
if (throwable instanceof MqttException)
{
transportException = PahoExceptionTranslator.convertToMqttException((MqttException) throwable, "Mqtt connection lost");
log.trace("Mqtt connection loss interpreted into transport exception", throwable);
}
else
{
transportException = new TransportException(throwable);
}

this.listener.onConnectionLost(transportException, this.connectionId);
transportException = PahoExceptionTranslator.convertToMqttException((MqttException) throwable, "Mqtt connection lost");
log.trace("Mqtt connection loss interpreted into transport exception", throwable);
}
else
{
transportException = new TransportException(throwable);
}

// Logging this at a debug level instead of warning or error because the IoThubTransport
// level will log it to warning or error if applicable.
log.debug("Mqtt connection lost", transportException);

this.listener.onConnectionLost(transportException, this.connectionId);
}

/**
Expand Down