Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow using multiple custom annotations for interception #54

Open
oakkitten opened this issue Feb 11, 2024 · 3 comments
Open

Allow using multiple custom annotations for interception #54

oakkitten opened this issue Feb 11, 2024 · 3 comments

Comments

@oakkitten
Copy link

This project seems to be well-suited for method call logging in debug builds. However, @Intercept is a bit vague a term, not reflecting the actual purpose of the annotation. Moreover, with debug logging, I can usually have trace- and debug-level log statements for different verbosity levels. So I would like to have something like @DebugLog and @TraceLog, and I would like to know which one was used in the listener.

(I wonder if it's feasible to pass the annotation object itself to the listener, if retained at runtiume. This would allow using annotations with parameters!)

@milis92
Copy link
Owner

milis92 commented Feb 22, 2024

Hayo, thanks for the issue.

While I agree that @Intercept might not be the best name for the runtime annotation, I don't think adding *Log annotations is a good idea.
Krangs purpose is not necessarily logging; it is a general instrumentation library.

Now, what I can do is:

  • Deprecate the @Intercept annotation
  • Replace the @Intercept with the @Trace annotation (not sold on the name 100%; feel free to add your suggestions)
  • Introduce an optional tracing context as the parameter for the @Trace annotation so that you can pass arbitrary data to Krang.

With the new tracing context you could do something like:

@Trace(context = [ "debug"])
fun test(){
}

//and later on on the intercept
Krang.addListener { functionName, parameters, context ->
    //here, you can check for the tracing context passed with the annotation
}

@oakkitten
Copy link
Author

Thank you for considering this.

I wonder if it would be feasible to allow creating custom annotations. I very much like the idea of a general-purpose instrumentation library, and I also like the word intercept—that's what is happening! If we could just choose our own names for the actual purposes.

Seeing how you can create annotations for other annotations, I imagine being able to write something along

@InterceptingAnnotation
annotation class DebugLog

and then just use it in the code. And, for ultimate versatility, I imagine that if this annotation is also annotated with @Retention(AnnotationRetention.RUNTIME), then the listener is provided with the annotation itself. Maybe use different listener types for cases where annotations are available at runtime, or just have it nullable, or even allow defining listeners for different (sets of) intercepting annotations?

@milis92
Copy link
Owner

milis92 commented Jun 29, 2024

Hey sorry for not replying sooner, got caught up with some other stuff. I was thinking about this and my proposal is as follows:

New name for the @Intercept annotation:

I'm voting for renaming@Intercept to @Trace. The reason is that contextually, @Trace makes more sense with what's happening under the hood, and it seems like it is a term that is widely used in tools similar to Krang

Custom annotations

Yeah, this is a good idea and should be a fairly straightforward thing to add.
In the future, you will be able to do:

@Trace 
annotation class DebugLog(val whatever: String)

@DebugLog(whatever = "whatever ")
fun something(){
}

// and later
Krang.addListener { name, arguments, traceContext ->

}

Where the traceContext is going to be part of the Krang runtime and will look like

// @param annotation will be either a @Trace or a custom annotation decorated with @Trace
data class TracingContext(annotation: Any)

Introducing trace context is a bit of future-proofing since in the next iterations I want to add a source code location of the function that is being instrumented, maybe even the time the function takes to execute, etc

Timeline

Now, I'm going to work on these changes after I finish the work required for Kotlin 2.0 - which should be straightforward now when the integration tests are done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants