Improve clarity and fix Docs for Logging

[dgomesbr]

What were you initially searching for in the docs?

• Don't use service_undefined in Java, use Null or String.Empty.
• Use units to represent the percentage, clarity over the 0.1. (http://jsr-108.sourceforge.net/javadoc/javax/units/NonSI.html#PERCENT) I'll review the javadocs of it to make it explicitly whether we should be using 0.01, 0.1, 10, etc.
• Message, Unserializable JSON values will be casted to string, question: How? calling toString()? if so, tell that toString() will be called. Also, what about exceptions there? Will that be swollen?
• Version is great, what about logging the alias?
• Please, make this a warning/info box, this is too subtle the way it is today and if this precedence is respected elsewhere, create a section for know before you go. Configuration on environment variable is given precedence over sampling rate configuration on annotation, provided it's in valid value range.

Is this related to an existing part of the documentation? Please share a link
https://awslabs.github.io/aws-lambda-powertools-java/core/logging/#standard-structured-keys

Describe how we could make it clearer
Address the points listed

If you have a proposed update, please share it here

[msailes]

I don't think it's possible to get the alias from the context - https://docs.aws.amazon.com/lambda/latest/dg/java-context.html
Do you have any other suggestions?

[msailes]

I think service_undefined works well here, it matches the python implementation and gives us a service name to use when we implement Metrics. We'll need a reasonable default for that core util as well.

[msailes]

For sampling, I'm happy with the String / Double approach. If this was a public method I think it would be best as a unit. But because it's an environment variable I think it would only increase complexity.

[dgomesbr]

I think service_undefined works well here, it matches the python implementation and gives us a service name to use when we implement Metrics. We'll need a reasonable default for that core util as well.

This is misleading, if we don't know the name, I would assume that is better to default to the FunctionName or even FunctionName:Version, until the customer properly configures it.

I don't think it's possible to get the alias from the context - https://docs.aws.amazon.com/lambda/latest/dg/java-context.html
Do you have any other suggestions?

getInvokedFunctionArn() – Returns the Amazon Resource Name (ARN) that's used to invoke the function. Indicates if the invoker specified a version number or alias.

For sampling, I'm happy with the String / Double approach. If this was a public method I think it would be best as a unit. But because it's an environment variable I think it would only increase complexity.

I think the point here is readability, if the argument is called samplingRate, I have to lookup/assume which is the actual unit, but this is totally style. Hopefully we have more inputs here.

[heitorlessa]

W.r.t - service_undefined provides an opportunity for the customer to fix it. Service has also a deeper meaning as it acts as a namespace when looking for data across multiple functions be that logs, metrics or traces.

As to context, I don't know about Java, but in the Python context object it'll fetch a FQDN -- If it's a given published version or alias it'll be the last value <function_name>:<version_or_alias>

[msailes]

I'd be really happy to look at a PR for adding alias information.

[dgomesbr]

W.r.t - service_undefined provides an opportunity for the customer to fix it. Service has also a deeper meaning as it acts as a namespace when looking for data across multiple functions be that logs, metrics or traces.

100%, but as soon as a customer incorporates that into multiple functions they will see service_undefined in multiple logstreams, or am I missing something? This goes hand-in-hand with tracing span naming which I'm in full agreement.

Now for the Alias, I'm seeking guidance on whether there's any logic behind or simply callee information.

[heitorlessa]

That is correct - Hence the emphasis on POWERTOOLS_SERVICE_NAME in the docs for all core utilities, including our blog post and examples. I’d be happy to log a warning if service name is not found. I would not use function name as the service name if not found as that will break many workflows, and their current understanding of service being a namespace they can search across core utilities too On the alias bit, it should be easy to test with a deployed function - Publish a version from the Console, and invoke that function from that version — It should reflect in the context object, as it’s injected by the bootstrap runtime process

…

On Fri, 4 Sep 2020 at 03:57, Diego Magalhães ***@***.***> wrote: W.r.t - service_undefined provides an opportunity for the customer to fix it. Service has also a deeper meaning as it acts as a namespace when looking for data across multiple functions be that logs, metrics or traces. 100%, but as soon as a customer incorporates that into multiple functions they will see service_undefined in multiple logstreams, or am I missing something? This goes hand-in-hand with tracing span naming which I'm in full agreement. Now for the Alias, I'm seeking guidance on whether there's any logic behind or simply callee information. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#69 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZPQBFAQ3MSOTX3KQKJCATSEBCQFANCNFSM4QRKWE3A> .

[msailes]

Alias as part of the context was something discussed in the aws-lambda-java-libs project.

aws/aws-lambda-java-libs#10

You might want to continue the discussion there.

[pankajagrawal16]

Message, Unserializable JSON values will be casted to string, question: How? calling toString()? if so, tell that toString() will be called. Also, what about exceptions there? Will that be swollen?

This should be fixed as part of #113

[pankajagrawal16]

@dgomesbr Can this issue be closed now based on some of the discussions above? 🤔

[dgomesbr]

For sure. Thank you all!