Investigate EXTRA_VALUE_AT_END Key Appearing in Various Log Entries #30
Comments
|
With non-JSON output, we can see |
|
This is due to errant log helper functions which accept |
bflad
added a commit
that referenced
this issue
Mar 4, 2022
…unctions to ...map[string]interface{}
Reference: #30
Reference: #31
Previously, implementers could introduce mismatched key-value pairs or expect `f` suffix-like string formatting of the message due to the final `...interface{}` variadic argument. Updating this to `...map[string]interface{}` has the following benefits:
- Calls that errantly were treating these as formatter functions will now receive a compiler error in the majority of cases (except parameters that already satisfied the `map[string]interface{}` type).
- Matched pairing is now enforced via the compiler, preventing occurence of go-hclog's `EXTRA_VALUE_AT_END` key in entries.
- Keys are now enforced to be `string`, where before they could be defined as other types and beholden to go-hclog's behavior of calling `fmt.Sprintf("%s", key)` resulting in likely unexpected keys for non-string types:
```
true: %!s(bool=true)
123: %!s(int=123)
123.456: %!s(float64=123.456)
[]string{"oops"}: [oops]
MyCustomType (without String() method): {}
```
Some disadvantages of this new approach include:
- Additional typing of the `map[string]T{}` for each call. Implementors can opt for `map[string]string` in many cases, or Go 1.18+ `map[string]any`, in preference over the potentially confusing/annoying `interface{}` typing.
- The burden of converting existing calls.
Pragmatically however, the advantages outweigh the disadvantages as this Go module does not yet have full compatibility promises (pre-1.0) and the direct dependencies are expected to grow exponentially in the near future when its existance is broadcasted more. Catching this critical API change earlier rather than later will have less of an effect on the ecosystem.
bflad
added a commit
that referenced
this issue
Mar 8, 2022
…unctions to ...map[string]interface{} (#34)
Reference: #30
Reference: #31
Previously, implementers could introduce mismatched key-value pairs or expect `f` suffix-like string formatting of the message due to the final `...interface{}` variadic argument. Updating this to `...map[string]interface{}` has the following benefits:
- Calls that errantly were treating these as formatter functions will now receive a compiler error in the majority of cases (except parameters that already satisfied the `map[string]interface{}` type).
- Matched pairing is now enforced via the compiler, preventing occurence of go-hclog's `EXTRA_VALUE_AT_END` key in entries.
- Keys are now enforced to be `string`, where before they could be defined as other types and beholden to go-hclog's behavior of calling `fmt.Sprintf("%s", key)` resulting in likely unexpected keys for non-string types:
```
true: %!s(bool=true)
123: %!s(int=123)
123.456: %!s(float64=123.456)
[]string{"oops"}: [oops]
MyCustomType (without String() method): {}
```
Some disadvantages of this new approach include:
- Additional typing of the `map[string]T{}` for each call. Implementors can opt for `map[string]string` in many cases, or Go 1.18+ `map[string]any`, in preference over the potentially confusing/annoying `interface{}` typing.
- The burden of converting existing calls.
Pragmatically however, the advantages outweigh the disadvantages as this Go module does not yet have full compatibility promises (pre-1.0) and the direct dependencies are expected to grow exponentially in the near future when its existance is broadcasted more. Catching this critical API change earlier rather than later will have less of an effect on the ecosystem.
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
terraform-plugin-log version
Relevant provider source code
Noticed with
tfsdklogusage of protocol logging, may also happen withtfsdkusage.Expected Behavior
Log entries should not extraneously contain the
EXTRA_VALUE_AT_ENDkey, which is injected byhc-logwhen it detects unpaired (key without value)Withusage, unless there is in fact an unpaired thing.Actual Behavior
Entries such as:
{ "@caller": "/Users/bflad/go/pkg/mod/github.com/hashicorp/terraform-plugin-go@v0.7.0/internal/logging/protocol.go:21", "@level": "trace", "@message": "Received request", "@module": "sdk.proto", "@timestamp": "2022-02-04T16:28:52.602080-05:00", "EXTRA_VALUE_AT_END": null, "tf_proto_version": "5.2", "tf_provider_addr": "", "tf_req_id": "a24b8a29-628a-01c2-0516-08b260627109", "tf_rpc": "GetProviderSchema" }This could be caused by a bug in
hc-log, a bug in this Go module, or buggy SDK/provider logger implementations. Let's rule out the first two. 😄Steps to Reproduce
TF_LOG_SDK_PROTO=trace terraform applyfrom a provider using terraform-plugin-go@v0.7.0.The text was updated successfully, but these errors were encountered: