I am for limiting types of attributes values only to UTF-8 strings and bytearrays. Also, I agree with Pavel, (2) is clear and without any reflection.
вт, 19 окт. 2021 г. в 14:18, Nikolay Izhikov <nizhi...@apache.org>: > I like (1) options. > > @ServiceRequestContextResource > private Function<String, Object> ctxFunc; > > Because, we use this style of API for injection of other resources - > logger, ignite instance, etc. > It may be confusing for the user to use several API styles for solving > similar tasks. > > > > 19 окт. 2021 г., в 11:04, Pavel Tupitsyn <ptupit...@apache.org> > написал(а): > > > > (2) seems to be the cleanest and most discoverable to me, > > also simpler to implement (no reflection necessary). > > > > But existing ServiceContext properties are for the entire instance, not > for > > the current call. > > So, to make it clear and obvious, let's do > > ServiceContext.currentCallContext().attribute(...). > > > > On Mon, Oct 18, 2021 at 7:20 PM Pavel Pereslegin <xxt...@gmail.com> > wrote: > > > >> Folks, > >> > >> I agree with Ivan that we can improve the user experience in Ignite > >> services by adding support for "middleware". > >> And as a first step, we need to pass the "caller context" to the > service. > >> > >> I see the following API options for reading this "context" inside a > >> service: > >> (please see "API proposal" section in Jira [1] for full formatted > examples) > >> > >> 1. Using a custom annotation (ServiceRequestContextResource) and > >> reading context attributes with a function. > >> > >> @ServiceRequestContextResource > >> private Function<String, Object> ctxFunc; > >> > >> public void serviceMethod() { > >> String login = (String)ctxFunc.apply("login"); > >> } > >> > >> 2. Using a new method of the existing ServiceContext. > >> > >> private ServiceContext svcCtx; > >> > >> public void init(ServiceContext svcCtx) { > >> this.svcCtx = svcCtx; > >> } > >> > >> public void serviceMethod() { > >> String login = svcCtx.attribute("login"); > >> // and/or > >> String login = (String)svcCtx.attributes().get("login"); > >> } > >> > >> > >> The next two options require wrapping Map<String, Object> into a new > >> ServiceRequestContext class. > >> > >> 3. Read context "wrapper" using special annotation and supplier. > >> > >> @ServiceRequestContextResource > >> private Supplier<ServiceRequestContext> ctxSupplier; > >> > >> public void serviceMethod() { > >> String login = ctxSupplier.get().attribute("login"); > >> } > >> > >> 4. Using the special static method of the "wrapper" class. > >> > >> public void serviceMethod() { > >> String login = > ServiceRequestContext.current().attribute("login"); > >> } > >> > >> Let's discuss which one is the way to go. > >> > >> [1] https://issues.apache.org/jira/browse/IGNITE-15572 > >> > >> вт, 12 окт. 2021 г. в 13:51, Ivan Daschinsky <ivanda...@gmail.com>: > >>> > >>> Hi, Val > >>> > >>>>> The examples you mentioned are more related to internal activities > >> (e.g., > >>>>> if authentication is handled by an Ignite server node, it can create > >> its > >>>>> internal context for a connection - this is certainly reasonable). > I'm > >>> only > >>>>> worried about exposing this to the end user. > >>> > >>> I'm talking about not Ignite auth, but external auth. Here I am > >> considering > >>> Ignite Service Grid as a microservice platform. > >>> Authentication microservice can be not related to Ignite at all, but > >> author > >>> of service may want to retrieve or authenticate user by user_id, that > is > >>> provided in request headers or context in jwt token, for example. > >>> > >>> The same is for tracing or metrics. Ignite internal mechanisms here > >> cannot > >>> help at all, because there is no context related to user's code. > >>> > >>> If we want to leave Ignite Service Grid as dump as possible, it is ok. > >> But > >>> therefore it cannot compete with more functional variants. > >>> > >>> But just adding request headers at first step and custom interceptors > >>> (client and server side) we can give to user's of Ignite Service Grid > a > >>> lot of opportunities. > >>> > >>> There is an example of golang grpc middlewares -- see how many > >> interesting > >>> use cases here: > >>> https://github.com/grpc-ecosystem/go-grpc-middleware > >>> > >>> вт, 12 окт. 2021 г. в 07:31, Valentin Kulichenko < > >>> valentin.kuliche...@gmail.com>: > >>> > >>>> Ivan, > >>>> > >>>> I'm a bit confused :) Unless I misread the initial suggestion, the > >> idea is > >>>> to provide a public API to create the context. In other words, it will > >> be > >>>> up to the end user to create this context properly, which affects the > >>>> business code - and that's exactly where I see an issue. > >>>> > >>>> The examples you mentioned are more related to internal activities > >> (e.g., > >>>> if authentication is handled by an Ignite server node, it can create > >> its > >>>> internal context for a connection - this is certainly reasonable). I'm > >> only > >>>> worried about exposing this to the end user. > >>>> > >>>> Maybe you can pick one of the use cases that you think would benefit > >> from > >>>> this feature the most, and provide a little more detail? How would you > >> like > >>>> to see the use case to be addressed and what is currently missing? > >>>> > >>>> Also, just to be clear: I'm not necessarily against the suggestion, > and > >>>> it's highly unlikely that I will want to veto it if you or someone > else > >>>> will decide to implement it. Just expressing my concerns. > >>>> > >>>> -Val > >>>> > >>>> On Sun, Oct 10, 2021 at 11:52 PM Nikolay Izhikov <nizhi...@apache.org > > > >>>> wrote: > >>>> > >>>>> +1 to have service proxy context. > >>>>> > >>>>>> 11 окт. 2021 г., в 09:43, Ivan Daschinsky <ivanda...@gmail.com> > >>>>> написал(а): > >>>>>> > >>>>>> Val, Pavel both of you are right, but on the other hand there are > >> some > >>>>>> other tasks > >>>>>> > >>>>>> 1. Distributed tracing. > >>>>>> 2. Custom metrics/measurements > >>>>>> 3. Auth and some related tasks (i.e. ingests full User info by > >> calling > >>>>> some > >>>>>> auth service in middleware). > >>>>>> > >>>>>> Do you both think that this is a good idea in business code? > >>>>>> > >>>>>> Without this functionality, our service grid cannot compete with > >> grpc > >>>> and > >>>>>> others as microservice framework, unfortunately. > >>>>>> > >>>>>> But if we introduce limited support for this "request headers", it > >> can > >>>>>> drastically improves this aspects of our service grid framework. > >>>>>> > >>>>>> > >>>>>> пн, 11 окт. 2021 г. в 00:48, Valentin Kulichenko < > >>>>>> valentin.kuliche...@gmail.com>: > >>>>>> > >>>>>>> I agree with Pavel. The suggested approach is indeed utilized > >> quite > >>>>>>> frequently, but it's inherently error-prone. > >>>>>>> > >>>>>>> The main issue is that it creates implicit assumptions about the > >>>>> behavior > >>>>>>> of both the service and the user's code. For example, if the > >> user's > >>>> code > >>>>>>> must provide a username, what if it doesn't? I assume it will get > >> an > >>>>> error, > >>>>>>> which is very counterintuitive. Even more importantly, how should > >> one > >>>>> learn > >>>>>>> about this requirement in the first place? It is not reflected in > >> the > >>>>> API > >>>>>>> in any way - and that's a big problem. > >>>>>>> > >>>>>>> The fact that the service implementor needs to update the API > >> methods > >>>>> when > >>>>>>> such requirements are introduced is actually a good thing, in my > >>>>> opinion. > >>>>>>> This forces the developer to stop and think about how the updated > >> API > >>>>>>> should look like and how to make sure it's backward-compatible (or > >>>> not, > >>>>> in > >>>>>>> case the new requirements are mandatory). Doing this through an > >>>> external > >>>>>>> context is basically the equivalent of saying "let the end user > >> deal > >>>>> with > >>>>>>> this". Not a good practice, in my view. > >>>>>>> > >>>>>>> Conversely, passing everything exclusively via method arguments > >>>>> guarantees > >>>>>>> that: > >>>>>>> > >>>>>>> - The user's code is always compliant with the service > >> contract. You > >>>>>>> can't "forget" to pass something to the service. > >>>>>>> - Any changes in the service contract (backward-compatible or > >>>>> otherwise) > >>>>>>> are explicitly reflected in the API. > >>>>>>> > >>>>>>> > >>>>>>> -Val > >>>>>>> > >>>>>>> > >>>>>>> On Sun, Oct 10, 2021 at 6:21 AM Pavel Tupitsyn < > >> ptupit...@apache.org> > >>>>>>> wrote: > >>>>>>> > >>>>>>>> Ivan, > >>>>>>>> > >>>>>>>> Yes, this approach is used by some other systems, and still, I > >> don't > >>>>> like > >>>>>>>> it very much. > >>>>>>>> Let's hear more opinions. > >>>>>>>> > >>>>>>>> On Sat, Oct 9, 2021 at 9:00 PM Ivan Daschinsky < > >> ivanda...@gmail.com> > >>>>>>>> wrote: > >>>>>>>> > >>>>>>>>> Hi. > >>>>>>>>> Pavel T., Ok, http rest dosn't have the clean design, in your > >>>> opinion. > >>>>>>>>> > >>>>>>>>> But what about grpc? The same? > >>>>>>>>> > >>>>>>>>> As for me, it is ok to pass additional parameters as list of > >>>> key-value > >>>>>>>>> pairs with keys as strings and values as bytearrays or strings. > >> It > >>>> is > >>>>>>> ok > >>>>>>>> to > >>>>>>>>> allow user to set up middlewares for services and allow to > >> enrich > >>>>>>> request > >>>>>>>>> context in this middlewares. It is very common approach > >> everywhere > >>>> and > >>>>>>> is > >>>>>>>>> very useful in distributed systems. The use cases are so > >> obvious, > >>>>>>> aren't > >>>>>>>>> they? > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> сб, 9 окт. 2021 г., 20:14 Pavel Tupitsyn <ptupit...@apache.org > >>> : > >>>>>>>>> > >>>>>>>>>> Pavel, > >>>>>>>>>> > >>>>>>>>>> Thanks for the explanation, I understand the use cases. > >>>>>>>>>> > >>>>>>>>>>> in REST service, he can set such parameters in request headers > >>>>>>>>>> > >>>>>>>>>> I don't consider HTTP-based services as a good example of a > >>>>>>>>>> clean architecture. > >>>>>>>>>> Data can be passed in URL parameters, in headers, and in body, > >> and > >>>>>>> each > >>>>>>>>> of > >>>>>>>>>> those ways has its own limitations. > >>>>>>>>>> There is no obvious correct way to do things. > >>>>>>>>>> > >>>>>>>>>>>> Ambient state is not obvious and the API looks confusing even > >>>>>>>> though I > >>>>>>>>>> understand our services stack quite well both in Java and .NET > >>>>>>>>>>> Can you clarify please? > >>>>>>>>>> > >>>>>>>>>> The proposed API adds a "side channel" for the data. > >>>>>>>>>> Some is passed as arguments, which is obvious, and some becomes > >>>>>>>> magically > >>>>>>>>>> available on the server side through some external context. > >>>>>>>>>> - You have to know about the context > >>>>>>>>>> - You have to understand that the context is only available > >> during > >>>>>>> the > >>>>>>>>>> method call (can't use it in some background logic) > >>>>>>>>>> > >>>>>>>>>> In my opinion, this is a bit too clever. I'm a fan of the > >>>> functional > >>>>>>>>>> programming approach where everything you need is passed as > >>>>>>> arguments. > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> On Fri, Oct 8, 2021 at 4:29 PM Pavel Pereslegin < > >> xxt...@gmail.com> > >>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>>> Igor, Pavel. > >>>>>>>>>>> > >>>>>>>>>>>> Why can not a user implement such context on application > >> level? I > >>>>>>>>>>> believe Ignite provides all necessary tools for that. > >>>>>>>>>>> The user wants to trace the source of the service call. For > >>>>>>> example, > >>>>>>>> a > >>>>>>>>>>> service must log the name of the user who made the calls of > >> the > >>>>>>>>>>> service. For now, there's no possibility to do that without > >>>>>>> modifying > >>>>>>>>>>> the service interface and implementation. Moreover, the user > >> must > >>>>>>>>>>> modify all methods of service to pass this parameter. For > >> example, > >>>>>>> in > >>>>>>>>>>> REST service, he can set such parameters in request headers, > >> why > >>>> we > >>>>>>>>>>> can't provide such usability in Ignite. > >>>>>>>>>>> > >>>>>>>>>>>> This will reduce the performance of all calls > >>>>>>>>>>> This feature is optional, if the context is not passed - then > >>>>>>> there's > >>>>>>>>>>> shouldn't be any performance difference. > >>>>>>>>>>> > >>>>>>>>>>>> Ambient state is not obvious and the API looks confusing even > >>>>>>>> though > >>>>>>>>> I > >>>>>>>>>>> understand our services stack quite well both in Java and .NET > >>>>>>>>>>> Can you clarify please? > >>>>>>>>>>> > >>>>>>>>>>> пт, 8 окт. 2021 г. в 15:46, Pavel Tupitsyn < > >> ptupit...@apache.org > >>>>> : > >>>>>>>>>>>> > >>>>>>>>>>>> Agree with Igor. > >>>>>>>>>>>> > >>>>>>>>>>>> I'm not sure this feature is a good fit for Ignite. > >>>>>>>>>>>> Ignite should not be responsible for such a high-level > >> concept, > >>>>>>>> this > >>>>>>>>>>> should > >>>>>>>>>>>> be on the application side instead. > >>>>>>>>>>>> > >>>>>>>>>>>> - As Eduard noted, it is hard to make this type-safe > >>>>>>>>>>>> - Ambient state is not obvious and the API looks confusing > >> even > >>>>>>>>> though > >>>>>>>>>> I > >>>>>>>>>>>> understand our services stack quite well both in Java and > >> .NET > >>>>>>>>>>>> - This will reduce the performance of all calls > >>>>>>>>>>>> > >>>>>>>>>>>> On Fri, Oct 8, 2021 at 3:44 PM Igor Sapego < > >> isap...@apache.org> > >>>>>>>>> wrote: > >>>>>>>>>>>> > >>>>>>>>>>>>> Hi guys, > >>>>>>>>>>>>> > >>>>>>>>>>>>> Why can not a user implement such context on application > >> level? > >>>>>>>>>>>>> I believe Ignite provides all necessary tools for that. User > >>>>>>> can > >>>>>>>>> just > >>>>>>>>>>>>> implement such a context as user type and pass it to > >> services > >>>>>>>> they > >>>>>>>>>>>>> need. Are the arguments why would Ignite need a separate > >>>>>>> feature > >>>>>>>>>>>>> for such a use case? > >>>>>>>>>>>>> > >>>>>>>>>>>>> Best Regards, > >>>>>>>>>>>>> Igor > >>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>>> On Fri, Oct 8, 2021 at 2:17 PM Eduard Rakhmankulov < > >>>>>>>>>>> erixon...@gmail.com> > >>>>>>>>>>>>> wrote: > >>>>>>>>>>>>> > >>>>>>>>>>>>>> I am not aware .NET capabilities, but as I can see service > >>>>>>> must > >>>>>>>>> be > >>>>>>>>>>>>>> implemented in *java* and even if can't serialize other > >> that > >>>>>>>> Map > >>>>>>>>> on > >>>>>>>>>>> .NET > >>>>>>>>>>>>>> side, on java side we can wrap this map with provided > >>>>>>>>> TypedContext > >>>>>>>>>>>>> (context > >>>>>>>>>>>>>> should be convertible from map in this case). > >>>>>>>>>>>>>> That leads to a situation when Java can use TypedContext > >> but > >>>>>>>>> other > >>>>>>>>>>>>> clients > >>>>>>>>>>>>>> can't. I believe that the majority of services users are > >>>>>>> using > >>>>>>>>> Java > >>>>>>>>>>> and > >>>>>>>>>>>>> it > >>>>>>>>>>>>>> should be taken in accordance. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> P.S. I think it is possible to send plain objects from .NET > >>>>>>>>> context > >>>>>>>>>>> to > >>>>>>>>>>>>>> cluster. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Best regards, Ed > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On Fri, 8 Oct 2021 at 14:40, Pavel Pereslegin < > >>>>>>>> xxt...@gmail.com> > >>>>>>>>>>> wrote: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Hi, Eduard! > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Thanks for your feedback. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> The idea sounds very good, but don't forget about the > >>>>>>>> platform > >>>>>>>>>>>>> services. > >>>>>>>>>>>>>>> For example, we may call Java service from .Net and > >>>>>>>> vice-versa. > >>>>>>>>>> I'm > >>>>>>>>>>>>>>> not sure if the context can be implemented as a custom > >>>>>>> class > >>>>>>>>>>> (instead > >>>>>>>>>>>>>>> of Map/Dictionary) in this case. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> пт, 8 окт. 2021 г. в 14:21, Eduard Rakhmankulov < > >>>>>>>>>>> erixon...@gmail.com>: > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Hi, Pavel > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Is it possible to provide type-safe API for > >>>>>>>>>> ServiceProxyContext ? > >>>>>>>>>>>>>>>> I think constructions like int arg1 = > >>>>>>>> ctx.attribute("arg1"); > >>>>>>>>>> are > >>>>>>>>>>>>> error > >>>>>>>>>>>>>>>> prone. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Can we make something like this : > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> //Signature with two generic params which allow the > >>>>>>>> compiler > >>>>>>>>> to > >>>>>>>>>>> check > >>>>>>>>>>>>>>>> if the service will be called with the wrong type > >>>>>>> context. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> public <T extends ContextedWith<CtxType>, CtxType> T > >>>>>>>>>>>>>>>> serviceProxyTyped(ClusterGroup prj, String name, Class<? > >>>>>>>>> super > >>>>>>>>>> T > >>>>>>>>>>>> > >>>>>>>>>>>>>>>> srvcCls, CtxType optCtx, boolean sticky, long timeout) > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> //new interface which services with scoped context should > >>>>>>>>>>> implement > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> public interface ContextedWith<T> { > >>>>>>>>>>>>>>>> T getCtx(); > >>>>>>>>>>>>>>>> } > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> // implementation can delegate to Map-like context or be > >>>>>>>>> POJO. > >>>>>>>>>>>>>>>> interface MyServiceContext { > >>>>>>>>>>>>>>>> int getArg1(); > >>>>>>>>>>>>>>>> String getUserId(); > >>>>>>>>>>>>>>>> } > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> class MyService implements > >>>>>>> ContextedWith<MyServiceContext> > >>>>>>>> { > >>>>>>>>>>>>>>>> void doThings() { > >>>>>>>>>>>>>>>> MyServiceContext ctx = getCtx(); > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> System.out.println("ctx.getArg1() = " + ctx.getArg1()); > >>>>>>>>>>>>>>>> } > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> @Override public MyServiceContext getCtx() { > >>>>>>>>>>>>>>>> return ServiceProxyContext.current(); > >>>>>>>>>>>>>>>> } > >>>>>>>>>>>>>>>> } > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> WDYT? > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Best regards, Ed. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> On Fri, 8 Oct 2021 at 13:26, Pavel Pereslegin < > >>>>>>>>>> xxt...@gmail.com> > >>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Hello Igniters! > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> I want to implement a feature to support a custom > >>>>>>>> "caller" > >>>>>>>>>>> context > >>>>>>>>>>>>> in > >>>>>>>>>>>>>>>>> ignite services (see example in ticket description > >>>>>>> [1]). > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Sometimes, when using Ignite services, it becomes > >>>>>>>> necessary > >>>>>>>>>> to > >>>>>>>>>>> pass > >>>>>>>>>>>>>>>>> custom parameters from the "request source" to the > >>>>>>>> service. > >>>>>>>>>>> This is > >>>>>>>>>>>>>>>>> most commonly used to track the origin of a service > >>>>>>> call > >>>>>>>>>> (user > >>>>>>>>>>> id, > >>>>>>>>>>>>>>>>> request id, session id eg see this user question [2]). > >>>>>>>>>>>>>>>>> At the moment, the only way to pass such parameters to > >>>>>>> a > >>>>>>>>>>> service is > >>>>>>>>>>>>>> by > >>>>>>>>>>>>>>>>> adding argument(s) to all called methods of the > >>>>>>> service, > >>>>>>>>>> which > >>>>>>>>>>>>> makes > >>>>>>>>>>>>>>>>> the code messy and also complicates development and > >>>>>>>>>>> maintenance. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> I propose letting the user set a custom context for the > >>>>>>>>>> service > >>>>>>>>>>>>> proxy > >>>>>>>>>>>>>>>>> and implicitly pass that context to the methods being > >>>>>>>>> called. > >>>>>>>>>>> This > >>>>>>>>>>>>>>>>> function should not affect the execution of service > >>>>>>>> methods > >>>>>>>>>> in > >>>>>>>>>>> any > >>>>>>>>>>>>>> way > >>>>>>>>>>>>>>>>> unless the user has specified a context. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> An example of using the proposed API [1]. > >>>>>>>>>>>>>>>>> PoC (except thin clients) [3]. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> WDYT? > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> [1] https://issues.apache.org/jira/browse/IGNITE-15572 > >>>>>>>>>>>>>>>>> [2] > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>> > >>>> > >> > https://stackoverflow.com/questions/57459071/apache-ignite-service-grid-service-call-context > >>>>>>>>>>>>>>>>> [3] https://github.com/apache/ignite/pull/9440 > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>>> > >>>>>> -- > >>>>>> Sincerely yours, Ivan Daschinskiy > >>>>> > >>>>> > >>>> > >>> > >>> > >>> -- > >>> Sincerely yours, Ivan Daschinskiy > >> > > -- Sincerely yours, Ivan Daschinskiy