OpcUaClient interface/inheritance & SessionInitializer

[peterfranciscook]

// (milo 0.6.16)
artifacts = [
        "org.eclipse.milo:sdk-client:0.6.16",
        "org.eclipse.milo:stack-core:0.6.16",
        "org.eclipse.milo:stack-client:0.6.16",
    ],

I've got a few classes in my OpcUa module that spin up a thread pool and do a some work when my app bootstraps (tree grokking, codec registration, node subscription, etc) and nothing after that.

I originally wrote these as guava idle services (... extends AbstractIdleService) but have begun refactoring them to implement the milo SessionInitializer interface instead.

Observation: the SessionInitializer::initialize method uses the UaStackClient interface rather than the UaClient interface.

• Does it delegate to the same underlying OpcUaClient object? After experimenting, the answer seemed to be "hard maybe"
  • Based on the observation that calling UaStackClient::getDynamicDataTypeManager in one of my SessionInitializer impl classes returns the same object as calling OpcUaClient::getDynamicDataTypeManager elsewhere in the code, I was inclined to think yes...
  • But based on the observation that instanceof checks are negative, the answer is a hard no

    @Override
    public CompletableFuture<Unit> initialize(UaStackClient uaStackClient, OpcUaSession session) {
        Logger.info("Initializing OpcUa codec registration...");
  
        boolean isOpcUaClient = false;
        boolean isUaClient = false;
        if (((Object) uaStackClient) instanceof OpcUaClient) {
            Logger.debug("uaStackClient instanceof OpcUaClient"); // not reached
            isOpcUaClient = true;
        }
        if (((Object) uaStackClient) instanceof UaClient) {
            Logger.debug("uaStackClient instanceof UaClient"); // not reached
            isUaClient = true;
        }
        if (!(isOpcUaClient || isUaClient)) {
            // only this code is reached
            Logger.debug("UaStackClient not instanceof OpcUaClient or UaClient");
        }
  
        // ...
    }
  

  • Inspecting the source code for the (concrete) OpcUaClient class more or less answers the question by revealing that it contains a UaStackClient delegate/member.

Apart from general curiosity, the reason I'd like to know more about it is I am currently hung up refactoring one of my AbstractIdleService as SessionInitializer because UaStackClient doesn't have the getAddressSpace method, which the service uses to grok the OpcUa tree (uses only 2 methods from AddressSpace - getNodeAsync(NodeId nodeId) and browseAsync(NodeId nodeId).

One route I haven't gone down is using Guice to fetch the @Singleton OpcUaClient instance from the injector because this will create a circular dependency as I add the SessionInitializers to the OpcUaClient in my module's @Provides (akin to a factory method if you're not familiar with Guice) methods for UaClient. Slightly better than that could be to inject a Provider<AddressSpace> into the SessionInitializer to delay initialization and break the circular dependency (the provider would have the UaClient injected, and return UaClient::getAddressSpace() from its get() method).

Before I get into the weeds with DI hacks, I'd like to know if there's some alternative API for browsing the tree with the UaStackClient instance passed to SessionInitializer::initialize

[kevinherron]

SessionInitializers receive a UaStackClient because they get called at a tricky time. The use case for these is literal: to initialize a Session, i.e. do anything else needed after it's activated, but before it's available from the SessionFsm.

So it doesn't receive an OpcUaClient because most, if not all, calls on OpcUaClient are going to call getSession(), and if you block you'll end up deadlocked.

I think they're of limited use to user application code because you have to be very careful about what you do in the implementation.

[peterfranciscook]

right - when I say bootstrapping here its really more like "now that the client has been instantiated, browse the tree for nodes relevant to certain classes, register codecs, subscribe to nodes we care about, and attach shared objects to the OpcUaSession"

[kevinherron]

SessionInitializers run at the wrong place for high level stuff like that. You should probably be thinking about doing this after the Session is activated.

I wanted to eliminate them all together for 1.0 but didn't manage it... hopefully some future 2.0 version 🙃 They're too much of a hack and dangerous to use.

Your use case needs a different solution but I don't know exactly what it is yet. We just do it by managing our client instance externally, doing what we need in the onSessionActive/Inactive calls, and preventing it from being used until "ready" by whatever terms we decide means it's ready.

[peterfranciscook]

SessionInitializers receive a UaStackClient because they get called at a tricky time. The use case for these is literal: to initialize a Session, i.e. do anything else needed after it's activated, but before it's available from the SessionFsm.

I did notice there seems to be pretty loose guarantees about timing/synchronization with SessionInitializer; I had to spin up an executor in one of my initializers to spin and wait for session.getAttribute(DataTypeTreeSessionInitializer.SESSION_ATTRIBUTE_KEY) to become available. It seemed to me that adding that initializer before the others (client.addSessionInitializer(new DataTypeTreeSessionInitializer());) ought to have been adequate to guarantee it was available by the time the next one started, but was wrong and had to hack around it

[kevinherron]

That's a little surprising because the initializers do run sequentially:

milo/opc-ua-sdk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/session/SessionFsmFactory.java

Lines 1446 to 1487 in 696bf00

	private static CompletableFuture<Unit> initialize(
	FsmContext<State, Event> ctx, OpcUaClient client, OpcUaSession session) {
	LinkedList<SessionFsm.SessionInitializer> initializers =
	new LinkedList<>(KEY_SESSION_INITIALIZERS.get(ctx).sessionInitializers);
	if (initializers.isEmpty()) {
	return completedFuture(Unit.VALUE);
	} else {
	return runSequentially(ctx, client, session, initializers);
	}
	}
	private static CompletableFuture<Unit> runSequentially(
	FsmContext<State, Event> ctx,
	OpcUaClient client,
	OpcUaSession session,
	LinkedList<SessionFsm.SessionInitializer> initializers) {
	if (initializers.isEmpty()) {
	return CompletableFuture.completedFuture(Unit.VALUE);
	} else {
	SessionFsm.SessionInitializer initializer = initializers.removeFirst();
	return initializer
	.initialize(client, session)
	.exceptionally(
	ex -> {
	try (MDCCloseable ignored =
	MDC.putCloseable("instance-id", ctx.getUserContext().toString())) {
	LOGGER.error(
	"Uncaught initialization error: {}",
	initializer.getClass().getSimpleName(),
	ex);
	}
	return Unit.VALUE;
	})
	.thenCompose(u -> runSequentially(ctx, client, session, initializers));
	}
	}

[peterfranciscook]

SessionInitializers run at the wrong place for high level stuff like that. You should probably be thinking about doing this after the Session is activated.

I wanted to eliminate them all together for 1.0 but didn't manage it... hopefully some future 2.0 version 🙃 They're too much of a hack and dangerous to use.

Your use case needs a different solution but I don't know exactly what it is yet. We just do it by managing our client instance externally, doing what we need in the onSessionActive/Inactive calls, and preventing it from being used until "ready" by whatever terms we decide means it's ready.

If you're soliciting suggestions for this/related mechanisms ... I'll throw this out there and feel free to ignore it entirely. Essentially, exposing callbacks for client lifecycle hooks in an OpcUaClient.Builder (I know that OpcUaClient.Builder doesn't actually exist) e.g. onStarting, onRunning, onConnected, onDisconnecting, onDisconnected, onReconnecting, onReconnected, onTerminated, onFailed, etc with something easily pluggable could benefit a lot of use cases. Plugins could either be ad-hoc interfaces (similar to SessionInitializer) or something more generic like Consumer<OpcUaClient> ...similar to the pattern used by the (same-same, but different MQTT vs OPCUA) java hivemq mqtt client/builder

[kevinherron]

@peterfranciscook perhaps something like this:

interface ClientLifecycleListener {

  /**
   * Called when {@link OpcUaClient#connect()} has been called, but the connection has not yet been
   * established.
   *
   * @param client the client that is connecting.
   */
  void onConnectBegin(OpcUaClient client);

  /**
   * Called after the Client has successfully connected to the Server and activated a Session.
   *
   * @param client the client that has connected successfully.
   */
  void onConnectSuccess(OpcUaClient client);

  /**
   * Called if the Client failed to connect to the Server or activate a Session.
   *
   * @param client the client that failed to connect.
   * @param failure the {@link Throwable} that caused the failure.
   */
  void onConnectFailure(OpcUaClient client, Throwable failure);

  /**
   * Called when {@link OpcUaClient#disconnect()} has been called, but the disconnection has not yet
   * completed.
   *
   * @param client the client that is disconnecting.
   */
  void onDisconnectBegin(OpcUaClient client);

  /**
   * Called after the Client has successfully disconnected from the Server.
   *
   * @param client the client that has disconnected successfully.
   */
  void onDisconnectSuccess(OpcUaClient client);

  /**
   * Called if the Client failed to disconnect cleanly from the Server.
   *
   * @param client the client that failed to disconnect.
   * @param failure a {@link Throwable} describing the cause of the failure.
   */
  void onDisconnectFailure(OpcUaClient client, Throwable failure);

  /**
   * Called when the Client is attempting to reconnect to the Server after a disconnection.
   *
   * <p>"Reconnecting" covers a broad range of actions, including re-establishing the secure
   * channel, re-activating the existing Session or creating a new Session, transferring
   * Subscriptions if necessary, etc...
   *
   * @param client the client that is attempting to reconnect.
   */
  void onReconnectBegin(OpcUaClient client);

  /**
   * Called after the Client has successfully reconnected to the Server and either re-activated or
   * created a new Session.
   *
   * @param client the client that has reconnected successfully.
   */
  void onReconnectSuccess(OpcUaClient client);

  /**
   * Called if the Client failed to reconnect to the Server or reactivate a Session.
   *
   * @param client the client that failed to reconnect.
   * @param failure a {@link Throwable} describing the cause of the failure.
   */
  void onReconnectFailure(OpcUaClient client, Throwable failure);

  /**
   * Called when the Client has lost its connection to the Server.
   *
   * <p>This may be due to a network failure, server shutdown, or other reasons.
   *
   * @param client the client that has lost its connection.
   * @param cause a {@link Throwable} describing the cause of the connection loss.
   */
  void onConnectionLost(OpcUaClient client, Throwable cause);
}

Though, I'm guessing what would be the most useful callbacks (onConnectSuccess, onReconnectSuccess, onConnectionLost) are approximated by using the existing onSessionActive and onSessionInactive.

[peterfranciscook]

I'm going to tinker with SessionActivityListener callbacks over the weekend and check back in with you. If there's TODO/feature work related to this, I'm happy to contribute

[kevinherron]

I may write this up in an issue along with some other Client changes I've been thinking about (splitting out an AbstractClient from OpcUaClient that houses all the service calls but doesn't do any Session or other high level stuff).

This would all be in 1.x though. The 0.6.x version line is dead.

[kevinherron]

Bleh, starting playing with this ClientLifecycleListener a bit but not really loving it. There's a lot of ambiguity trying to map these methods to the session lifecycle. Might just allow users to listen to the SessionFsm events and state transitions instead.

I do something similar with my netty-channel-fsm library, you end up with a callback interface like this:

  public interface TransitionListener {

    /**
     * A state transition has occurred.
     *
     * <p>Transitions may be internal, i.e. the {@code from} and {@code to} state are the same.
     *
     * @param from the {@link State} transitioned from.
     * @param to the {@link State} transitioned to.
     * @param via the {@link Event} that caused the transition.
     */
    void onStateTransition(State from, State to, Event via);

  }

You can infer a ton about the lifecycle of the client from watching the state transitions.
SessionFsm States:

enum State {
  Inactive,
  CreatingWait,
  Creating,
  Activating,
  Transferring,
  Initializing,
  Active,
  Closing,
  ReactivatingWait,
  Reactivating
}

[peterfranciscook]

Bleh, starting playing with this ClientLifecycleListener a bit but not really loving it. There's a lot of ambiguity trying to map these methods to the session lifecycle. Might just allow users to listen to the SessionFsm events and state transitions instead.

I do something similar with my netty-channel-fsm library, you end up with a callback interface like this:

  public interface TransitionListener {

    /**
     * A state transition has occurred.
     *
     * <p>Transitions may be internal, i.e. the {@code from} and {@code to} state are the same.
     *
     * @param from the {@link State} transitioned from.
     * @param to the {@link State} transitioned to.
     * @param via the {@link Event} that caused the transition.
     */
    void onStateTransition(State from, State to, Event via);

  }

maybe something closer to Guava's Service.Listener interface would be friendlier?

[peterfranciscook]

You can infer a ton about the lifecycle of the client from watching the state transitions. SessionFsm States:

enum State {
  Inactive,
  CreatingWait,
  Creating,
  Activating,
  Transferring,
  Initializing,
  Active,
  Closing,
  ReactivatingWait,
  Reactivating
}

Remembered a feature of hivemq's client library re: disconnect events that may also be a relevant to this - the registered disconnected callback gets a "context" object with some useful info that can be used to determine if its a "normal" disconnect vs client issue vs server issue

https://github.com/hivemq/hivemq-mqtt-client/blob/a7ae5ca957f444c8c121357553b9953979faa8d8/src/main/java/com/hivemq/client/mqtt/lifecycle/MqttClientDisconnectedContext.java#L30

Their mqtt client's connection state (as opposed to overall client state) has its own enum that captures some subtle/edge states {'DISCONNECTED', 'CONNECTING', 'CONNECTED', 'DISCONNECTED_RECONNECT', 'CONNECTING_RECONNECT'}

[peterfranciscook]

My feedback on writing an implementation of SessionActivityListener for app-level (as opposed to milo client) startup tasks via the onSessionActive callback.

The onSessionActive callback/method is passed a UaSession object, which doesn't contain a reference to the OpcUaClient it belongs to (classes that implement SessionInitializer at least get a UaStackClient).

Since my (short-lived / one-shot) initializer classes/services need an OpcUaClient to query the tree at startup, it needs to be passed to the constructors of those classes (by whatever means; I use DI). Since I use a factory** method to create a preconfigured singleton/shared OpcUaClient instance, this creates a circular dependency* between the SessionActivityListenerImpl and my factory method for OpcUaClient. There's a few ways to break the cycle here (I chose guice Provider injection), so it's not a no-go, but seems like a better way might be possible

*i.e.

**well, basically a factory method, it's a guice Provider