Service definition for OOP adapter that will be on session plan. #406

guptasu · 2018-03-13T20:09:41Z

As per the API design https://docs.google.com/document/d/1Fic1Mgp0W5Sxhd0BxAFOc82DbSYWTsYpi5P-pv8ZjSQ/edit

geeknoid · 2018-03-14T02:24:09Z

mixer/adapter/model/v1beta1/service.proto

+import "google/protobuf/any.proto";
+import "google/rpc/status.proto";
+
+// `AdapterService` is implemented by backends that wants to provide telemetry and policy functionality to Mixer as an


Can we call this InfrastructureService instead?

These things are not adapters, they're just services.

I agree they are not adapters. However, I feel Infrastructure is also not super helpful; But don't have any other suggestions. And I also think we can drop the suffix Service.

How about few options that intent do imply Istio integration backends.

InfrastructureIntegration ?

InfrastructureIntegrationBackend ?

IstioIntegration ?

IstioIntegrationBackend ?

I like #1 because it is uses the terms we commonly use around Istio and is yet generic enough to make back-ends feel they are not completely locked into Istio.

@mandarjog @douglas-reid for their opinions

I agree they are not adapters. However, I feel Infrastructure is also not super helpful; But don't have any other suggestions. And I also think we can drop the suffix Service.

How about few options that intent do imply Istio integration backends.

InfrastructureIntegration ?

InfrastructureIntegrationBackend ?

IstioIntegration ?

IstioIntegrationBackend ?

I like #1 because it is uses the terms we commonly use around Istio and is yet generic enough to make back-ends feel they are not completely locked into Istio.

@mandarjog @douglas-reid for their opinions
Thoughts ?

geeknoid · 2018-03-14T02:25:32Z

mixer/adapter/model/v1beta1/service.proto

+//
+// For a given `AdapterService`, Mixer calls the `Validate` function, followed by `CreateSession`. The `session_id`
+// returned from `CreateSession` is used to make subsequent request-time Handle calls and the eventual `CloseSession` function.
+// Mixer assumes that, given the `session_id`, the backend can retrive the necessary context to serve the


geeknoid · 2018-03-14T02:27:09Z

mixer/adapter/model/v1beta1/service.proto

+    // The `session_id` provides the Handle functions a way to retrive the necessary configuration associated with the session.
+    // Upon Mixer configuration change, Mixer will re-invoke `CreateSession` for all handler configurations whose existing
+    // sessions are invalidated or didn't existed.
+    rpc  CreateSession(CreateSessionRequest) returns (CreateSessionResponse);


We should mention that a backend is allowed to return the same session id if given the same config block. This would happen when multiple instances of Mixer in a deployment all create sessions with the same config.

Note that given CloseSession, reusing session_id assumes the backend is doing refcounting.

geeknoid · 2018-03-14T02:27:35Z

mixer/adapter/model/v1beta1/service.proto

+    rpc  CreateSession(CreateSessionRequest) returns (CreateSessionResponse);
+
+    // Closes the session associated with the `session_id`. Mixer closes a session when its associated handler
+    // configuration or the instance configuration changes. Backend is suppose to cleanup all the resources associated


As per the API design https://docs.google.com/document/d/1Fic1Mgp0W5Sxhd0BxAFOc82DbSYWTsYpi5P-pv8ZjSQ/edit

guptasu

Thanks @geeknoid for the review. PTAL at the changes. Also added some suggestions on the service name

guptasu · 2018-03-14T18:31:24Z

mixer/adapter/model/v1beta1/service.proto

+//
+// For a given `AdapterService`, Mixer calls the `Validate` function, followed by `CreateSession`. The `session_id`
+// returned from `CreateSession` is used to make subsequent request-time Handle calls and the eventual `CloseSession` function.
+// Mixer assumes that, given the `session_id`, the backend can retrive the necessary context to serve the


guptasu · 2018-03-14T18:33:46Z

mixer/adapter/model/v1beta1/service.proto

+    // The `session_id` provides the Handle functions a way to retrive the necessary configuration associated with the session.
+    // Upon Mixer configuration change, Mixer will re-invoke `CreateSession` for all handler configurations whose existing
+    // sessions are invalidated or didn't existed.
+    rpc  CreateSession(CreateSessionRequest) returns (CreateSessionResponse);


guptasu · 2018-03-14T18:34:08Z

mixer/adapter/model/v1beta1/service.proto

+    rpc  CreateSession(CreateSessionRequest) returns (CreateSessionResponse);
+
+    // Closes the session associated with the `session_id`. Mixer closes a session when its associated handler
+    // configuration or the instance configuration changes. Backend is suppose to cleanup all the resources associated


guptasu · 2018-03-14T18:36:02Z

mixer/adapter/model/v1beta1/service.proto

+import "google/protobuf/any.proto";
+import "google/rpc/status.proto";
+
+// `AdapterService` is implemented by backends that wants to provide telemetry and policy functionality to Mixer as an


I agree they are not adapters. However, I feel Infrastructure is also not super helpful; But don't have any other suggestions. And I also think we can drop the suffix Service.

How about few options that intent do imply Istio integration backends.

InfrastructureIntegration ?

InfrastructureIntegrationBackend ?

IstioIntegration ?

IstioIntegrationBackend ?

I like #1 because it is uses the terms we commonly use around Istio and is yet generic enough to make back-ends feel they are not completely locked into Istio.

@mandarjog @douglas-reid for their opinions
Thoughts ?

geeknoid · 2018-03-14T21:30:25Z

We've generally referred to these things as "infrastructure backends", so I think that would make a fine name. So I'll put my vote down for #5:

InfrastructureBackend

geeknoid · 2018-03-14T22:58:38Z

mixer/adapter/model/v1beta1/service.proto

+    //
+    // If the backend couldn't create a session for a specific handler configuration and
+    // returns non S_OK status, Mixer will not make request-time Handle calls associated with that handler configuration.
+


Remove empty line.

geeknoid · 2018-03-14T22:58:56Z

mixer/adapter/model/v1beta1/service.proto

@@ -0,0 +1,117 @@
+// Copyright 2018 Istio Authors


Maybe rename the file?

guptasu · 2018-03-14T23:07:43Z

@mandarjog over to you now for second lgtm. PTAL.

guptasu · 2018-03-15T20:48:00Z

@mandarjog I am getting this in, since it is needed by my next PRs. Feel free to leave comment and I can address them in a separate PR. Thanks.

guptasu · 2018-03-21T19:03:19Z

@douglas-reid I sneaked this in while you were out. Feel free to suggest improvements if you see anything that is not right. I can definitely use as many reviews as possible on this external API definition. @mandarjog you too :-). It is a lot easy to change stuff now than later.

Automatic merge from submit-queue. [DO NOT MERGE] Auto PR to update dependencies of mixerclient This PR will be merged automatically once checks are successful. ```release-note none ```

guptasu requested review from theganyo, mandarjog, wora, douglas-reid and geeknoid March 13, 2018 20:09

googlebot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Mar 13, 2018

geeknoid reviewed Mar 14, 2018

View reviewed changes

guptasu added 2 commits March 14, 2018 12:07

Service definition for OOP adapter that will be on session plan.

c305651

As per the API design https://docs.google.com/document/d/1Fic1Mgp0W5Sxhd0BxAFOc82DbSYWTsYpi5P-pv8ZjSQ/edit

Address Martin's feedback.

27126f5

guptasu force-pushed the oopadapterservice branch from 420b77a to 27126f5 Compare March 14, 2018 19:08

guptasu commented Mar 14, 2018

View reviewed changes

rename service to InfrastructureBackend

d40fed4

geeknoid approved these changes Mar 14, 2018

View reviewed changes

Rename file

896c90a

guptasu merged commit 88ec95d into istio:master Mar 15, 2018

guptasu deleted the oopadapterservice branch March 15, 2018 20:48

nacx pushed a commit to nacx/api that referenced this pull request Feb 23, 2022

Defer permissions check on GetTelemetrySources (istio#406)

55bc93a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Service definition for OOP adapter that will be on session plan. #406

Service definition for OOP adapter that will be on session plan. #406

guptasu commented Mar 13, 2018

geeknoid Mar 14, 2018

guptasu Mar 14, 2018 •

edited

Loading

guptasu Mar 14, 2018 •

edited

Loading

geeknoid Mar 14, 2018

guptasu Mar 14, 2018

geeknoid Mar 14, 2018

geeknoid Mar 14, 2018

guptasu Mar 14, 2018

geeknoid Mar 14, 2018

guptasu Mar 14, 2018

guptasu left a comment

guptasu Mar 14, 2018

guptasu Mar 14, 2018

guptasu Mar 14, 2018

guptasu Mar 14, 2018 •

edited

Loading

geeknoid commented Mar 14, 2018

geeknoid Mar 14, 2018

geeknoid Mar 14, 2018

guptasu commented Mar 14, 2018

guptasu commented Mar 15, 2018

guptasu commented Mar 21, 2018

Service definition for OOP adapter that will be on session plan. #406

Service definition for OOP adapter that will be on session plan. #406

Conversation

guptasu commented Mar 13, 2018

Choose a reason for hiding this comment

guptasu Mar 14, 2018 • edited Loading

Choose a reason for hiding this comment

guptasu Mar 14, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guptasu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guptasu Mar 14, 2018 • edited Loading

Choose a reason for hiding this comment

geeknoid commented Mar 14, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guptasu commented Mar 14, 2018

guptasu commented Mar 15, 2018

guptasu commented Mar 21, 2018

guptasu Mar 14, 2018 •

edited

Loading

guptasu Mar 14, 2018 •

edited

Loading

guptasu Mar 14, 2018 •

edited

Loading