Set default values for tracing utilty parameters

[RobertoC27]

Is your feature request related to a problem? Please describe.
Changing the default captureResult param for the tracing utility to "false" would make my life easier, given that most of my methods require having it turned off because it causes errors while interacting with other services.

Describe the solution you'd like
It would be nice to be able to set the captureResult property for the tracing annotation to false and let all methods called inherit that default instead of having to put capture=false on every single annotation

Describe alternatives you've considered
right now I override the params on every annotation

Additional context
I'm working with a library that fails when tracing's capture response is true

[heitorlessa]

Would an environment variable help here?

That way you wouldn't change your code, and yet override the default -- it's something we've been thinking for the Python Powertools, at least.

For our own learning, could you share more on what services you're working with that this breaks?

We know S3 is one of them

[RobertoC27]

Yes, env vars make sense for this! I'm working with Cloudhsm, using the JCE. Curious that you mention S3, since that has not given us any issues with capture response.

[pankajagrawal16]

Agreed, the environment variable will probably be best here following how other utilities behave as well.

[pankajagrawal16]

I looked a bit into the implementation aspect of this. Few immediate thoughts, Since Java annotation won't allow Boolean as an attribute type, we might have to take the annotation way. In order to distinguish if its a default value or user has specified it before honoring any specific environment variable, we need something which is not primitive like true/false.

Practically, that would mean something like:

@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.METHOD)
public @interface PowertoolsTracing {
    String namespace() default "";
    @Deprecated
    boolean captureResponse() default true;
    @Deprecated
    boolean captureError() default true;
    CaptureMode mode() default CaptureMode.ENVIRONMENT_VAR;
}

public enum CaptureMode {
    RESPONSE,
    ERROR,
    RESPONSE_AND_ERROR,
    ENVIRONMENT_VAR
}

The default value of mode can be ENVIRONMENT_VAR and which can in turn default to true if no env variable is specified.

Pros:

• Requested feature can be supported in a clean way and also gives possibility of adding other modes if thats needed ever(right now cannot think of any though)

Cons:

• Will be a breaking change for users if they are overriding the default values today once we remove the deprecated attributes.

Unless I am missing something completely obvious and this can be solved in simpler way, Will be interesting to hear thoughts from others too. Partial implementation to set things in perspective.

[RobertoC27]

you are right, for us this would be a breaking change. no doubt but in the end it leads to a cleaner code.
Am i understanding this is how it would be or am i wrong?

// Current way of handling things
@Tracing (captureResponse=false)
public void someMethod(int param, ...)

@Tracing (captureResponse=false)
public void noErrorCapture(int param, ...)

@Tracing (captureError=false)
public void anotherMethod(int param, ...)

@Tracing
public void extraMethod(int param, ...)

// Proposed way
@Tracing
@CaptureMode(mode="ENVIRONMENT_VAR")
public void someMethod(int param, ...)

@CaptureMode(mode="ENVIRONMENT_VAR")
public void noErrorCapture(int param, ...)

@Tracing 
@CaptureMode(mode="RESPONSE")
public void anotherMethod(int param, ...)

@Tracing 
@CaptureMode(mode="RESPONSE_AND_ERROR") //this would be optional since default mode is capture both, right?
public void extraMethod(int param, ...)

[pankajagrawal16]

Yeah like this, except that the mode value is an enum and strongly typed instead of a String.

Also, the default value will be ENVIRONMENT_VAR which will default capture response and error to true if no environment variable specified. This practically means, if users today are using the default configuration, they won't need to change anything.

If users want to change the default behavior, they can just override the 2 new environment variable to whatever value they want.

If users want to override behavior for some of the users only, then they can choose any one of the supported modes.

Hope this makes sense? @RobertoC27

[RobertoC27]

If users want to change the default behavior, they can just override the 2 new environment variable to whatever value they want.

one final question, is there going to be 1 env var per behavior?

[pankajagrawal16]

Yeah, At least that's how I have thought about it so far here

[RobertoC27]

makes sense, since combining them would make more complex overriding them down the line.

[pankajagrawal16]

Hi @RobertoC27 , This is now available in v1.2.0. Do try out and let us know in case of any feedback/issues.

[RobertoC27]

Hey @pankajagrawal16
We finally got to migrate our lambdas to this new version and its is working with no issues so far, thanks!

[pankajagrawal16]

Hey @pankajagrawal16
We finally got to migrate our lambdas to this new version and its is working with no issues so far, thanks!

Hey @RobertoC27, Great to hear!