Add API for tracking usage of wrapper SDKs #273
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -138,6 +138,7 @@ h3(#restclient). RestClient | |
| *** @(RSC7d6)@ Libraries may offer a @ClientOptions#agents@ property, for use only by other Ably-authored SDKs, on a need-to-have basis. | ||
| **** @(RSC7d6a)@ The product/version key-value pairs supplied to that property should be injected into all @Agent@ library identifiers emitted by connections made as a result of REST or Realtime instances created using those @ClientOptions@. | ||
| **** @(RSC7d6b)@ An API commentary must be provided for this property. This commentary must make it clear that this interface is only to be used by Ably-authored SDKs. | ||
| *** @(RSC7d7)@ To _attribute a given REST request to a given wrapper SDK_ means to, given a @WrapperSDKProxyOptions@ object describing the wrapper SDK, inject the product/version key-value pairs contained in these options' @agents@ property into the @Agent@ library identifier emitted for that REST request. "@WP6@":#WP6 defines the circumstances in which a REST request must be attributed to a wrapper SDK. | ||
| * @(RSC18)@ If @ClientOptions#tls@ is true, then all communication is over HTTPS. If false, all communication is over HTTP however "Basic Auth":https://en.wikipedia.org/wiki/Basic_access_authentication over HTTP will result in an error as private keys cannot be submitted over an insecure connection. See @Auth@ below | ||
| * @(RSC8)@ Supports two protocols: | ||
| ** @(RSC8a)@ "MessagePack":https://msgpack.org/ binary protocol (this is the default for environments having a suitable level or support for binary data) | ||
|
|
@@ -194,6 +195,10 @@ h3(#restclient). RestClient | |
| ** @(RSC22b)@ Returns an array of @BatchResult<BatchPublishSuccessResult | BatchPublishFailureResult>@s. Optionally, in languages where this is idiomatic, an overload may be implemented whereby the method can be called with a single @BatchPublishSpec@ and return a single @BatchResult<BatchPublishSuccessResult | BatchPublishFailureResult>@. This is not a feature of the REST API, whose response will still be an array, so if implementing this overload, the SDK will have to extract the element from the array. | ||
| ** @(RSC23)@ This clause has been replaced by "@RSC24@":#RSC24. It was valid up to and including specification @2.1@. | ||
| ** @(RSC24)@ @RestClient#batchPresence@ function takes an array of channel name strings and sends them as a comma separated string in the @channels@ query parameter in a GET request to @/presence@, returning a @BatchResult<BatchPresenceSuccessResult | BatchPresenceFailureResult>@ object. | ||
| * @(RSC26)@ @RestClient#createWrapperSDKProxy@: | ||
| ** @(RSC26a)@ Returns a wrapper SDK proxy client whose underlying client is this @RestClient@. See "@WP1@":#WP1 for definitions of these terms. | ||
| ** @(RSC26b)@ Accepts a single argument, a @WrapperSDKProxyOptions@. | ||
| ** @(RSC26c)@ An API commentary must be provided for this method. This commentary must make it clear that this interface is only to be used by Ably-authored SDKs. | ||
|
|
||
| h3(#rest-auth). Auth | ||
|
|
||
|
|
@@ -464,6 +469,11 @@ h3(#realtimeclient). RealtimeClient | |
| ** @(RTC17a)@ Returns the current value of the @#clientId@ attribute of the @RealtimeClient@ object’s @#auth@ attribute | ||
| * @(RTC10)@ The client library should never register any listeners for internal use with the public @EventEmitter@ interfaces (such as @Connection#on@) or message/event subscription interfaces (such as @RealtimeChannel#subscribe@) in such a way that a user of the library calling @Connection#off()@ or @RealtimeChannel#unsubscribe()@ to remove all listeners would result in the library not working as expected | ||
| * @(RTC11)@ Unexpected internal exceptions, as defined in "@RSC20@":#RSC20, must be handled as gracefully as possible and reported to Ably's error reporting service when enabled. The aim when handling unexpected exceptions should be to ensure that no invalid or inconsistent state can potentially be left after handling the exception; depending on circumstances the remedial action could include failing the transport, failing the connection, rejecting a message, reinitialising the library completely, etc. | ||
| * @(RTC14)@ @RealtimeClient#createWrapperSDKProxy@: | ||
| ** @(RTC14a)@ Returns a wrapper SDK proxy client whose underlying client is this @RealtimeClient@. See "@WP1@":#WP1 for definitions of these terms. | ||
| ** @(RTC14b)@ Accepts a single argument, a @WrapperSDKProxyOptions@. | ||
| ** @(RTC14c)@ An API commentary must be provided for this method. This commentary must make it clear that this interface is only to be used by Ably-authored SDKs. | ||
|
|
||
|
|
||
| h3(#realtime-connection). Connection | ||
|
|
||
|
|
@@ -899,6 +909,27 @@ h3(#backoff-jitter). Incremental backoff and jitter | |
| h3(#realtime-compatibility). Forwards compatibility | ||
| * @(RTF1)@ The library must apply the "robustness principle":https://en.wikipedia.org/wiki/Robustness_principle in its processing of requests and responses with the Ably system. In particular, deserialization of ProtocolMessages and related types, and associated enums, must be tolerant to unrecognised attributes or enum values. Such unrecognised values must be ignored. | ||
|
|
||
| h3(#wrapper-sdk-proxy). Wrapper SDK proxy client | ||
|
|
||
| A _wrapper SDK_ is an Ably-authored non-core SDK. | ||
|
|
||
| The core SDK provides an API for wrapper SDKs to supply Ably with analytics information that allows us track the usage of these SDKs. | ||
|
|
||
|
There was a problem hiding this comment. Another thing that comes to mind - in Chat we have a use-case whereby the SDK agents are changed "on the fly" (not part of the constructor). This is in the React case, whereby we set the agents when a client is passed to the React component. Is this something that's supported here / needs further speccing? There was a problem hiding this comment. That's not a use case that I was aware of. We'd probably want to have an API that allows you to mutate the There was a problem hiding this comment. It's probably worth specifying, if only with a comment that says it primarily pertains to React-like circumstances. There was a problem hiding this comment. Do we really want to mutate the agent? I’d rather think of a proxy as a decorator (those two GoF patterns are almost identical). Now we have Can we use Does that make sense? There was a problem hiding this comment. Thinking about this some more, and I'm wondering whether what we could do instead in chat-js is, upon realising that we're operating in a React context, discard the initial wrapper SDK proxy client that we created upon instantiation of the chat client, and call There was a problem hiding this comment.
That could work, right now React calls an underlying method on the chat client to "add the agent". That method could just get a new wrapper instance. As getting a new wrapper instance still wraps the same realtime client, we can just replace the wrapper in the one place that'll need it. There was a problem hiding this comment. @ttypic I addressed your composition question in #273 (comment) There was a problem hiding this comment. Ah @ttypic I just realised that we replied to Andy at about the same time; #273 (comment) wasn't intended to be a reply to #273 (comment) 🙂 |
||
| * @(WP1)@ A _wrapper SDK proxy client_ is a client that is created by calling the @createWrapperSDKProxy@ method on an instance (hereafter referred to as the _underlying client_) of @RestClient@ ("@RSC26@":#RSC26) or @RealtimeClient@ ("@RTC14@":#RTC14). It is expected that a wrapper SDK proxy client will only be created by a wrapper SDK. | ||
| * @(WP2)@ A wrapper SDK proxy client must provide the same public API (that is, methods and properties) as its underlying client. Calling a method or accessing a property of a wrapper SDK proxy client should behave as if the same action had been performed directly on the underlying client, except for some modified usage tracking behaviour which is described in "@WP6@":#WP6 and "@WP7@":#WP7. | ||
| * @(WP3)@ This specification does not prescribe how a wrapper SDK proxy client should be implemented nor its concrete type. An implementation may choose to replace the return value of methods and properties with an implementation-defined type that differs from that returned by the underlying client; for example, a wrapper SDK proxy client's @channels@ property may return an object of some class which wraps the underlying @Channels@ instance and forwards all method calls to it. And, in turn, this type's @#get@ method may return an object of a class which wraps the underlying @RestChannel@ or @RealtimeChannel@ class and forwards all method calls to it. | ||
| * @(WP4)@ A wrapper SDK proxy client should not provide a @createWrapperSDKProxy@ method. | ||
|
There was a problem hiding this comment. Why do you think we should prevent this? I think with React-case it can be useful. Also if you can do this, I believe it shows that implementation has good design There was a problem hiding this comment. I think that it could be hard to implement, because — at least in the ably-cocoa implementation — the wrapper SDK proxy client relies on private API of the underlying client; that is, it wouldn't work if the underlying client only conformed to the public interface. There was a problem hiding this comment. (I mean, we could make the wrapper SDK proxy client and the public |
||
| * @(WP5)@ A wrapper SDK proxy client does not have any state of its own; rather, it shares all state with its underlying client. | ||
| * @(WP6)@ When the following methods are called via a wrapper SDK proxy client, then the resulting REST request must be attributed to the wrapper SDK (as defined in "@RSC7d7@":#RSC7d7), using the @WrapperSDKProxyOptions@ that were used to create the wrapper SDK proxy client: | ||
| ** @(WP6a)@ @RestChannel#publish@ | ||
|
There was a problem hiding this comment. Do we need a specific list of methods? Would we not want to make it so for any methods that interact with the server? There was a problem hiding this comment. The list here is based on Matt saying in ADR-117 that we're interested in tracking billable operations. I agree that it would be nice to try and write the spec here in a more general fashion, but I'm struggling to think of how we'd express it. Because initially we'd try and say something like "all REST requests initiated via a wrapper SDK proxy should be attributed to that wrapper SDK", but there are some REST requests where the meaning of "initiated via a wrapper SDK proxy" gets a bit muddy when the underlying client is not being used exclusively by a single wrapper SDK, because the requests happen as part of some common process that is difficult to attribute to a single user action, for example token requests or the REST calls involved in registering a device for push notifications. I guess we could explicitly exclude these REST calls from the "all REST requests initiated via a wrapper SDK proxy should be attributed to that wrapper SDK" spec point. There was a problem hiding this comment. Gonna have a think about how we can re-word this one (I agree it's not great as is, means that we have to keep making sure that we update this list each time we add a new billable operation REST API). |
||
| ** @(WP6b)@ @RestClient#batchPublish@ and @RealtimeClient#batchPublish@ | ||
| ** @(WP6c)@ @RestPresence#get@, and @first@ and @next@ on the @PaginatedResult@ that it returns | ||
| ** @(WP6d)@ @RestClient#batchPresence@ and @RealtimeClient#batchPresence@ | ||
| ** @(WP6e)@ @RestChannel#history@ and @RealtimeChannel#history@, and @first@ and @next@ on the @PaginatedResult@ that they return | ||
| ** @(WP6f)@ @RestPresence#history@ and @RealtimePresence#history@, and @first@ and @next@ on the @PaginatedResult@ that they return | ||
| ** @(WP6g)@ @RestClient#request@ and @RealtimeClient#request@, and @first@ and @next@ on the @HttpPaginatedResponse@ that they return | ||
| * @(WP7)@ If the @WrapperSDKProxyOptions@ that were used to create the wrapper proxy client has a non-null @agents@ property, then, when the "@RTS3@":#RTS3 @#channels#get@ method is called on a wrapper proxy SDK client whose underlying client is a @RealtimeClient@, the wrapper proxy client should behave as if @#get@ had been called with a channel options formed by adding an additional @agent@ key to the @params@ of the passed channel options. The value for this key should be formed using the same algorithm as used to form the @Agent@ library identifier in "@RSC7d1@":RSC7d1, using the key-value entries from the @agents@ property of the @WrapperSDKProxyOptions@ that were used to create the wrapper SDK proxy client, and _no other entries_ (i.e. disregard the "this must include at least the …" from RSC7d1). | ||
|
|
||
| h2(#state-conditions-and-operations). State conditions and operations | ||
|
|
||
| h3(#connection-states-operations). @Connection.state@ effects on realtime operations | ||
|
|
@@ -1775,6 +1806,11 @@ h4. CipherParamOptions | |
| ** @(CO2c)@ @keyLength@ (optional) integer - the length in bits of the @key@; for example 128 or 256 | ||
| ** @(C02d)@ @mode@ (optional) string – the cipher mode; currently the only supported non-null value is @CBC@ | ||
|
|
||
| h4. WrapperSDKProxyOptions | ||
| * @(WPO1)@ Options for controlling the creation of a wrapper SDK proxy client ("@WP1@":#WP1). | ||
| * @(WPO2)@ Has the following attributes: | ||
| ** @(WPO2a)@ @agents@ @[String: String?]?@ - a set of agents describing the wrapper SDK which is creating the proxy client. This property has the same semantics as the @ClientOptions#agents@ property. | ||
|
|
||
| h3(#types-push). Push notifications | ||
|
|
||
| h4. PushChannelSubscription | ||
|
|
@@ -1870,6 +1906,10 @@ class RestClient: // RSC* | |
| batchPublish(BatchPublishSpec) => io BatchResult<BatchPublishSuccessResult | BatchPublishFailureResult> // RSC22 | ||
| batchPublish(BatchPublishSpec[]) => io BatchResult<BatchPublishSuccessResult | BatchPublishFailureResult>[] // RSC22 | ||
| batchPresence(string[]) => io BatchResult<BatchPresenceSuccessResult | BatchPresenceFailureResult> // RSC24 | ||
| createWrapperSDKProxy(WrapperSDKProxyOptions) => RestClientInterface // RSC26 | ||
|
|
||
| interface RestClientInterface // WP* | ||
| // This is an interface that has the same instance methods as RestClient, except for createWrapperSDKProxy. | ||
|
|
||
| class RealtimeClient: // RTC* | ||
| constructor(keyOrTokenStr: String) // RTC12 | ||
|
|
@@ -1900,6 +1940,10 @@ class RealtimeClient: // RTC* | |
| batchPublish(BatchPublishSpec) => io BatchResult<BatchPublishSuccessResult | BatchPublishFailureResult> // RSC22 | ||
| batchPublish(BatchPublishSpec[]) => io BatchResult<BatchPublishSuccessResult | BatchPublishFailureResult>[] // RSC22 | ||
| batchPresence(string[]) => io BatchResult<BatchPresenceSuccessResult | BatchPresenceFailureResult> // RSC24 | ||
| createWrapperSDKProxy(WrapperSDKProxyOptions) => RealtimeClientInterface // RTC14 | ||
|
|
||
| interface RealtimeClientInterface // WP* | ||
| // This is an interface that has the same instance methods as RealtimeClient, except for createWrapperSDKProxy. | ||
|
|
||
| class ClientOptions: // TO* | ||
| embeds AuthOptions // This is not currently documented in the spec and needs to be – see https://github.com/ably/docs/issues/1476 | ||
|
|
@@ -2487,6 +2531,9 @@ class TokenRevocationSuccessResult: | |
| class TokenRevocationFailureResult: | ||
| target: string // TRF2a | ||
| error: ErrorInfo // TRF2b | ||
|
|
||
| class WrapperSDKProxyOptions: | ||
| agents: [String: String?]? // WPO2a | ||
| </pre> | ||
|
|
||
| h2(#old-specs). Old specs | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something that we should perhaps consider here - right now, connections to the server are made by the underlying SDK, and because
autoconnect=truewe never see connections from the wrapper SDKs.Obviously with autoconnect, that's not something we can realistically do when the option is turned on. But perhaps we should decide if we want to provide wrapper details in connections in any way, and also specify it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, in my assumptions of the requirements in ADR-117 I assumed that we did not want to attribute Realtime connection events to a wrapper SDK. This was based on the assumption that
autoconnectwould be used, though. But, if you look at Matt's response to my question about what we'd like to track, he seems to believe that connections are not something we are interested in tracking.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah perfect, that's already been discussed :) Yeah I agree attachments are more valuable than just connections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think tracking connections is fundamentally incorrect in terms of analytics. The connection is initiated by
RealtimeClient, so it shouldn’t be counted as Chat SDK usage. Otherwise, the wrapper SDK usage will obscure allRealtimeClientusage, leading to incorrect assumptionsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah exactly, all of our current chat analytics are based on attachments + frontend API requests anyway