[Proposal]: Interceptors #7009
Comments
|
Can you spec out the paths, esp vis a vis unix vs windows paths? Specifically, this seems problematic:
Unless the compiler is already normalizing these paths across platform. |
I worry we're doing this at the wrong time. This only seems to be for methods, when we might want it for properties, etc. should this wait for 'extensions' to be done first? |
I don't see this as any morally wrong versus calling one method and another being called. :) |
Would prefer that this break up the type-name and member-name into separate pieces. |
|
There's a lot to digest here but I'm trying to reconcile how this proposal compares to the AOP scenarios that I often face. At first glance this seems really limited, especially if only one interception can happen at a time. It also seems that the interception cannot "wrap" the target, at least not unless the target is itself accessible from the interceptor. There's no access to the underlying joinpoint, which would make aspects like logging, tracing, transactions, bulkheading, etc. possible. This seems to be very specialized around the specialization of framework code. I have other concerns about how this is very much spooky action at a distance, as in looking at a PR you can no longer tell if two identical method invocations back to back actually call the same method. There's also the fragility that if the generator doesn't run the interception becomes broken. |
|
To me this feels extremely limited and niche, and at the same time introduces a lot of moving parts that need to be kept in sync. There also doesn't seem to be a way to opt in or out of it for different invocations, the only construct I can think of would be comments like the The obvious question I have though is how would this work with default parameters? Would the interceptor need to know the default value and mirror it, or would that be passed through even though the caller did not specify it? |
|
If I read this correctly I think this is basically replace/original on the call-site. There's #3992 to help with that but this doesn't seem to make it easier?
You can already do this (sort of): [AddLogging]
public partial void Method();
private void Method_Impl() { .. }I think it comes down to a not-so-hacky way to do either of those, interceptors is definitely an option but still kind of hacky IMO. The first case is like an overload sensitive to constant arguments. That's pure/constexpr which can also interact with generators: // could evaluate to 3 at compile-time, provided all arguments are constant and `Add` is pure.
var result = Add(1, 2);// could evaluate the argument at compile-time provided it's a constant, using source generators.
var result = Evaluate(expressoin: "1+2");
var result = IsMatch(str, pattern: "\d+");
// generate a specialized IsMatch for each constant pattern
partial bool IsMatch(string, const string pattern)The second case is kind of like a horizontal extension for C {
public new void Method() { .. } // preferred over the original C.Method
}I've filed dotnet/roslyn#20631 for this a while back - right now it can be silently ignored. |
|
The biggest drawback is probably spooky action at a distance. I understand that by marking my method interceptable I acquiesce to having my logic replaced, but now the user of my library Foo can reference another library Bar that silently generates interceptors for my method that do whatever they want. |
I don't necessarily agree that the first sentence is the best way to achieve the goal in the second sentence. If there was an API to make direct edits, I would imagine that the compiler could still generate some kind of serialized patch containing those edits which it could save to disk and read later. While that would be pretty fragile in the event of source changes, it wouldn't be any more fragile than referencing a character offset, and it might avoid needing to re-parse/bind everything in the (probably more typical) case when you aren't saving/reading the modifications to/from disk. |
|
The more I think about this proposal the less I think it addresses more general-purpose AOP scenarios. I use AOP heavily in my Java services. We routinely use aspects for cross cutting concerns. Those concerns include tracing, logging, metrics, bulk heading, circuit breaking, concurrent call rates, retries (with exponential backoff), memoization, transactions and declarative security. I've also written more specific aspects to facilitate instrumentation of third-party libraries. This interceptor solution may cover some of those cases, but even then it seems it would be a non-ideal way to accomplish it. It doesn't handle other commonly requested scenarios at all, such as IPNC, since the interception happens at the call-site. There's nothing wrong with call-site AOP, but it's only one flavor and can only handle certain use cases. Worse, this proposal explicitly specifies that multiple interceptors aren't legal, but that's an incredibly common case in my code. I honestly don't see what use cases this proposal is intended to satisfy, except the extraordinarily narrow case of allowing ASP.NET to replace user-written code with their own. It's also been suggested that this is an appropriate route to implement optimization, where runtime-provided source generators will rewrite some percentage of the user-written code with more runtime-optimized (but certainly less readable) versions. That feels 100% backwards to me, and still very narrow. Not to mention, such "optimizations" can only ever benefit people using newer versions of C# with support for these intercepting generators. Every other language is left out. Coming from languages where post-processing and byte-code rewriting are a normal thing, this is extremely bizarre to suggest as a language enhancement. |
|
How would |
|
Thank you for the feedback so far.
Note that the compiler doesn't require paths to have any particular form. They just have to be representable in a .NET string. You can really think of this as: imagine the method we are calling has a CallerFilePath parameter. What string would we pass there? The string we put in the InterceptsLocationAttribute needs to exactly match that string in order to intercept that call. If the implicit CallerFilePath string argument differs based on which environment we are compiling in, then the argument to InterceptsLocation needs to change in the exact same way. This is very problematic for any manually-written InterceptsLocationAttribute--it's automatically non-portable, unless very specific pathmap arguments are used--for example, have no idea if the right thing will just start happening if you pathmap
Great question. It does feel like if we wanted to do this, it should be possible to do it with pretty much any non-type member, unless we had a specific reason for leaving them out. This tends to raise the question about how to denote the "location" of a usage of any kind of member, in a way that is not ambiguous with the usage of any other member. For example, use of implicit conversion operators might be tough to intercept based on location. #7009 (comment) #7009 (comment)
I definitely want us to consider if the use case of the platform specializing calls to platform methods within a project is better served by an post-compilation IL rewriting step. That will require revisiting any reasons we've been reluctant to introduce such a step in the past. It's clear that disallowing composition of multiple interceptors is really limiting here. As we look a little more broadly at this space I would definitely like for us to consider what it would mean to allow composition. The sequencing definitely seems easiest to define when you also control the original definition of the thing whose calls will get replaced.
I think this would require the "syntactic marker at interception site".
The interceptor would not need to know or mirror the default value, but it would need to have a parameter corresponding to that optional parameter--just as we disallow implicitly converting the method group of
Although it's hard to have duplicate file names in a real world project, there's no guarantee of unique paths in the compiler itself. (This was a significant complicating factor for file-local types.) Most likely, if a single tuple of
Yeah, we would potentially want something like "Key tokens" listed under "Alternatives" to reduce the risk of this.
I think this is a great point. I would like to try looking at the problem from a different angle, where we consider if some controlled facility could be introduced to make certain extensions better matches than instance methods in some cases. Could this give us some of the benefits of replace/original while still being able to work across projects--that is, where the replacement isn't in the same project as the original. I don't yet know in which cases this would be or what drawbacks this would have. There could be some fairly spooky problems where the replacement method wouldn't actually get used in places where we want it to. What I am seeing is that some scenarios like ASP.NET minimal API or query providers want to dispatch based on location essentially because some information about lambda expressions is lost after compilation or requires reflection or complex traversal to access at runtime. But other places want to dispatch based on:
Basically, what if a specialized implementation of a method could be added to a project, which dispatches however the implementer wants, simply by using the flow-control constructs of the language, rather than sticking declarative references to source locations into attributes on the signature. Consider the following extremely strawman sample, which hopefully gets the basic idea across. IRouteBuilder builder = ...;
// optionally: "conversion" to an explicit-extension type? where the framework exposes some path for doing this.
SpecializedRouteBuilder builder1 = builder.WithOpportunisticSpecialization();
builder1.MapGet("/products", (request, response) => ...);
builder1.MapGet("/items", (request, response) => ...);
builder1.MapGet("/config", (request, response) => ...);
// WebApp.generated.cs
// make it an optimization problem?
explicit extension SpecializedRouteBuilder for IRouteBuilder
{
// add an extension which deliberately wins over the one actually present in the type?
public shadowing IRouteBuilder MapGet(string route, Delegate handler,
[CallerFilePath] string filePath = null,
[CallerLineNumber] int lineNumber = -1,
[CallerColumnNumber] int columnNumber = -1)
{
switch (filePath, lineNumber, columnNumber)
{
// assume Native AOT can inline and constant-propagate to ensure
// this dispatch doesn't actually need to happen at runtime in most cases.
case ("file.cs", 1, 1): Method1(route, handler); return this;
case ("file.cs", 2, 2): Method2(route, handler); return this;
case ("file.cs", 3, 3): Method3(route, handler); return this;
case ("file.cs", 4, 4): Method4(route, handler); return this;
case ("file.cs", 5, 5): Method5(route, handler); return this;
case ("file.cs", 6, 6): Method6(route, handler); return this;
default: base.MapGet(route, handler);
}
}
}
// Regex.generated.cs
explicit extension MyRegexFactory for Regex
{
// add an extension which deliberately wins over the one actually present in the type?
public static shadowing Regex IsMatch(string pattern)
{
switch (pattern)
{
case ("a+"): return APlusMatcher();
case ("ab+"): return ABPlusMatcher();
default: return base.IsMatch(pattern);
}
}
} |
I think it’s a common desire to decorate a property in some way and get the boilerplate for implementing |
If I understood the extension proposal correctly, an
I don't think merely the "location" is the defining factor here (just like the Of course, this isn't workable for typeof, lambdas or generics (basically anything that doesn't have a For generics, it could be a case for specialization: extension for Serializer {
// `new` makes sure that this is specialization/shadowing so the signature must match.
public static new string Serialize</*const*/ MyType>() { .. }
}Which can be enabled for constants as well: extension for Regex {
public static new bool IsMatch(string str, const string pattern = "abc") { .. }
}These would result in a separate method and eliminate the need for a switch. None of that would work for lambdas but I think it should remain an implementation detail, that is, to use some auxiliary APIs to help with generating the |
It was mentioned on Discord that this is because IL-rewriting is challenging and the tools aren't up to snuff, and I don't doubt it. Regardless of these use cases I think there's a good opportunity for the .NET teams to come together and provide such tooling, that can emit appropriate IL and debugging symbols. For the most part, writing transformers and aspects In the Java world is a pretty simple experience. It's extremely rare to emit raw byte code, you typically write normal Java classes to describe the advice and use either annotations or a fluent DSL to describe where that advice will be applied. Tooling in IDEs like IntelliJ will show you where the advice will be applied. Transformation can then be performed as a post-compilation step, prior to a native/AOT build. It's pretty slick and easy to use once you grok the concepts of AOP and interception. I would imagine an official solution for .NET, with tooling support, could very easily rival it, too. |
I think if the receiver of the member access is the extension type, then members on the extension type are a better match than members on the underlying type. But if the receiver is the underlying type of an implicit extension, then the implicit extension members are only considered after we try and fail to find an applicable member on the underlying type. e.g. var mt = new MyType();
mt.M(); // MyType.M();
mt.M1(); // Ex.M();
Ex ex = mt;
ex.M(); // Ex.M();
public class MyType
{
public void M() { }
}
implicit extension Ex for MyType
{
public void M() { }
public void M1() { }
} |
I think to account for that it could be required to add specializations/interceptors for a particular member only in a single extension declaration in addition to a marker on the original member. class C {
public replaceable void Method() { .. } // permits replacement by a generator
}
extension for C {
public replace void Method() { .. } // but if this already exists, no one else can replace it
}That's assuming an external replacement could be unambiguous to begin with. |
|
Please please please call this ComeFromAttribute. The opportunity is too perfect! |
This is quite a bit like what is described in #7061, except that we were considering only letting extensions get shadowed/replaced by extensions. Instead of a |
|
Is it considered that the interceptor receives a reference to the original method through the class Original
{
public void Caller()
{
Console.WriteLine(this.Interceptable1(1)); // prints '6'
}
[Interceptable]
public int Interceptable1(int x)
{
return x + 1;
}
}
class Interceptor
{
[InterceptsLocation(...)]
public int Intercepter1(int x, Func<int, int> original)
{
return original(x + 1) * 2;
}
} |
|
This feature seems to address a very specific problem with a very specific solution. While this may be effective in the cases at hand, it may also be problematic in the long run, as the C# compiler is expected to last many more decades. Instead of creating a highly specialized solution at the compiler level, it may be worth considering a more general and flexible approach based on aspect-oriented programming (AOP). AOP has been used successfully in various domains for over 20 years and offers many benefits for modularizing cross-cutting concerns. At PostSharp Technologies, we have been developing Metalama, an AOP framework based on Roslyn. Metalama still does not support call-site aspects (i.e., interceptors), but we have plans to implement them in the future. We believe that Metalama could meet the requirements of this feature proposal and provide a more elegant and robust solution. We would be happy to collaborate with you on this matter and share our expertise and insights. |
|
Does this work safely even if the code-cleanup automatically replaces spaces with tabs or vice versa? |
|
@ufcpp yes :-) |
|
what did i miss? using System;
using System.Runtime.CompilerServices;
var c = new C();
c.InterceptableMethod(1); // prints `interceptor 1`
c.InterceptableMethod(1); // prints `other interceptor 1`
c.InterceptableMethod(1); // prints `interceptable 1`
class C
{
public static Action<int> Interceptable { get;set; } = (param) => Console.WriteLine($"interceptable {param}");
public Action<int> InterceptableMethod { get; set; } = Interceptable;
}
static class D
{
[ModuleInitializer]
public static void Init() {
Func<Action<int>> gen = () => {
var originalMethod = C.Interceptable;
var actions = new Action<int>[] {
param => Console.WriteLine($"interceptor {param}"),
param => Console.WriteLine($"other interceptor {param}"),
originalMethod
};
var index = 0;
return (param) => {
actions[index](param);
if (index < actions.Length - 1) index++;
};
};
C.Interceptable = gen();
}
} |
And is PostSharp Technology going to put the AOP features into C# as a core feature set? Or should your link have went to the pricing page? https://www.postsharp.net/metalama/pricing Until that happens, it is not a solution worth mentioning here on the Core DotNet issue trackers. What's next? We should be paying some third party vendors for basic language features like generics / expression tree / etc? Or is it dotnet team's position to have gaps in the language feature set so third party vendors can profit off it? Be sure to let us all know either way. This is a specific solution to a specific problem many of us here face, and having this implemented will solve many of our specific problems. |
I believe the point here is that a proper AOP solution will cover not only this use case but many, many others that interceptors cannot handle at all. Obviously any solution that ends up built into the language/compiler will not require developers to license/purchase third-party components. |
I'm not sure that's possible; to modify the example immediately preceding this statement: class C<T>
{
private T someField;
public static void Use(...)
{
var x1 = something./*L1*/Interceptable("a", someField);
var x2 = something./*L2*/Interceptable("a", someField);
}
}An interceptor cannot magic away the fact that there's an unresolved |
|
The feature seems be working. The proof of concept is here. |
Not sure exactly what problem that's attempting to solve... can you elaborate? |
|
In this topic, our colleagues expressed some doubts that this feature could be useful for AOP implementation. I wanted to check it and so far I don’t see any problems. |
Right, I get that you tried to dot that I or cross that T... I mean in the example itself, elaborate on the steps, what is considered "existing", how is it "intercepting", at every step, "what value is it adding", and so forth... Thank you... |
Roslyn interceptors are not designed to implement AOP. You are right that they can be useful for AOP, especially when combined with source generators and analyzers, but the vanilla Roslyn is not yet sufficient to implement a full AOP framework. Roslyn interceptors are call-site, i.e., they will modify the caller. With AOP, most of the time, you will want to advise the method implementation. You will also want to advise not only methods but also fields, properties, constructors, and events; you will want to introduce members and attributes to the type even when it is not Therefore, even with interceptors, we would not be able to build our AOP framework based on the vanilla Roslyn. We must still maintain our own fork that adds source transformers as an extension point. Once you have the extension points, you still need to implement several abstraction layers to build an actual AOP framework to handle abstraction to the C# level of thinking, aspect composition (what happens when several aspects are added to the same target), quantification (an API allowing devs to add aspects to targets) to name a few. |
|
@gfraiteur, I understand your point. Classic AOP assumes that modification of a method is done by the method developer himself and this is his will. Interceptors modify a method at the time of its call at the will of the method's consumer. Which, by the way, is also a valid scenario. |
|
Except for the cases where the interception needs have access to internal details of the type, or needs to occur somewhere within the implementation. There are use-cases for both call-site and implementation AOP, and some overlap between the two. The problem is that this approach can only handle this particular flavor, in this particular way, and there is no opportunity for it to "grow up" into a more comprehensive solution. |
|
@HaloFour, access to internal details can technically be solved by making their types partial. I agree with everything else. Interceptors are not AOP, but a simulation of AOP, which can be quite useful anyway. |
|
To a point, yes. But I would argue that it would make the plumbing awkward and probably brittle for many common AOP use cases, like IPNC. It's a hacky solution to a niche problem vs. considering a sustainable path. Coming from Java, where AOP is common and normalized in several of the major frameworks, it a massive disappointment. |
|
I wouldn't consider it as a disappointment. Other way around, I think this is big progress. About 15 years ago some MVP guys presented to Mr. Hejlsberg how metaprogramming works in other languages and he said that this is TOO BIG GUN. Today we are discussing Source Generators. What is this if not progress? :) Another 15 years and we will get... |
|
Lets call it Duck AOP. If it walks like an AOP and it quacks like an AOP, then it must be an AOP. |
A better topic for Discord. 😀 Is it progress? Sure. It's something. But the fact that it isn't thinking of that next step, and it has these very narrow specific use cases in mind, doesn't give me a lot of confidence that they are thinking about further progress. As with source generators, it scratches around the edges, but doesn't make real inroads.
A duck with one leg and no bill. |
The problem is, at least from GitHub, I don't see any long term plan on this feature (interceptors). It instead looks more like an ad-hoc solution to a specific problem encountered recently by ASP.NET team. The problem is C# has a high standard for compatibility, which is why this feature receives a lot of downvotes. There are many open issues here on GitHub that have been open for years, which I would think are better examples for "progress". As has been mentioned above, source generators cannot modify method body and therefore cannot be used as an AOP solution. |
|
Besides modifying a method body... which I maintain is a security liability. I don't know any CTO, myself included, who is going to go for that. Why does anyone need to do that. Again, I refer to the NHibernate Interceptors solution, or just the NHibernate code generation besides, which introduces proxy classes. Which, IMO, is what you want; avert having to modify an author's code. That's just bad form. If you have that as an AOP, then stipulate the requirements of introducing an intermediate wrapper proxy, i.e. things being |
You don't know any CTOs of companies that use Java with Spring? The proxy approach has a lot of downsides, including performance overhead and the inability to intercept calls within the type to itself. Not to mention, what needs to be instrumented might be internal. Doesn't work with native compilation either. Proxying is fine for the simplest of cases, and is a lot better than interceptors, but it still falls short. Notably, Java is currently working on an official transformation API to make bytecode manipulation a first-class citizen. Anyway, I think this is a better conversation for Discord. My opinion here is pretty well known. 😄 |
I hope Microsoft is listening... |
I think the team messaging has been pretty clear in this area... along the lines of, there might have been a day when feature parity, if familiarity of langauge features, was a higher priority. today, moving forward, as I understand it, not so much. These are not my sentiments, but coming from Mads and the language team, in any of the more recent interviews I have observed. Which is to say, if you want to replace byte codes, then perhaps consider the Java environment. Maybe there is a use case for this sort of invasive AOP, but I don't see it. Or it needs to be brought forward what that is. At least in its current form, the proposal is most certainly a liability none of my shops would be willing to assume. I mean, I can write a All due respect, FWIW... Good luck, good hunting! |
Observability.
It's not a question of achieving feature parity with Java, it's a question of providing solution to common problems. |
Or language familiarity. It's not that common of a problem. Or there are other, better, more acceptable, and kosher, ways of doing that. Again, referring to the NHibernate proxy pattern. |
Not being a problem for you doesn't mean that it isn't a problem for others. There are numerous requests for language features that either request AOP or would be easily solved via AOP. IPNC and DependencyProperties are high on that list. The current additive source generator solutions aren't sufficient.
Manual, invasive, error-prone, slow, incompatible with native compilation. Or, in the case of interceptors, requiring you to allow third-parties to integrate their build tools into your build process in order to even work at all. Disagreement noted, we won't find common ground here. Note, I would prefer that Roslyn supported an AST translation model vs. bytecode rewriting. One that would emit the effective source that would continue to work with tooling. But there are big advantages to AOP via rewriting, namely that it isn't compiler/toolchain specific. My aspects, written in pure Java with declarative annotations to define pointcuts, work across any JVM language. |
As someone who has written both an IL-based and Roslyn-based AOP framework, I can say that IL rewriting belongs to the past. The only case I can see is runtime instrumentation, based on the profiling API.
We've been serving thousands of customers for over 15 years with PostSharp and have collected some telemetry data. I can confidently say that the top 25% of PostSharp users make 15% of code savings. So, AOP, when used properly, can reduce code by 15%. And not only code but also bugs. Project that on a couple of millions of C# developers and their annual salary, and you get the worth of a proper AOP framework for the industry in the long term if Microsoft properly embraces AOP instead of implementing ad-hoc and short term solutions every time they need one -- which they have done for the last two decades. As for the use cases, there are many:
AOP excels at adding these features to source code after release 1.0, without requiring an architecture change. |
|
I can see this being very useful if the interceptor method can get a reference to the method call that it is replacing. That would enable interceptors to wrap logic around the original call. |
Agree 💯 on that point. |
|
your interceptor method can be an extension method. so it can be passed the original instance object the method was being called off of. And, of course, the method you emit can then emit a call to the original. |
That would only work if the method in question is public or internal(?). |
|
@11clock those are the only things i would expect externally generated code to be able to call. If you want to have private access, generate into a different partial part of the current type. |
|
I can see that interceptor support for properties has been listed as "may come in the future". I'm experimenting with the feature and I'm loving it so far. Support for interceptors on properties would be awesome. I'm hoping to just throw a not implemented exception on the properties and rely on an interceptor to provide the appropriate implementation. Till then I'll just make my properties invoke a method that I can intercept. Of course, a lot better solution would probably be partial properties, but that doesn't seem like something we are going to be getting any time soon. Keep up the great work! ❤️❤️ |
|
@CyrusNajmabadi Any news on the plans for Interceptors for .Net 9/10? On the Interceptors feature |
|
Interceptors are still experimental. They may change radically, or be removed at a future point. You can use, but plan accordingly. |
and others
Interceptors
Feature specification has been moved to https://github.com/dotnet/roslyn/blob/main/docs/features/interceptors.md. As this is not currently implemented as a language feature, we will not be tracking specific status pieces on this issue.
Design meetings
The text was updated successfully, but these errors were encountered: