dbt-core icon indicating copy to clipboard operation
dbt-core copied to clipboard

[CT-756] Stringify user-provided messages to `{{ log() }}`

Open jtcohen6 opened this issue 2 years ago • 1 comments

Reproduction case

-- models/any_model.sql

{% set some_list = ['a', 'b', 'c'] %}

{{ log(some_list, info = true) }}

select 1 as id
$ DBT_ENV_SECRET_WHATEVER=1234 dbt run
12:12:30  Running with dbt=1.2.0-a1
12:12:30  Encountered an error:
'list' object has no attribute 'replace'

I'm not even using the secret, but because I've defined one, dbt is searching log messages to scrub out any instances of the secret value. This error is not related to the recent change re: secret rendering. It's just related to how we're scrubbing secrets from log messages in general, and have been since v1.0.0:

https://github.com/dbt-labs/dbt-core/blob/6e8388c653c6bbeee934aea7db93730bd88df091/core/dbt/events/functions.py#L122-L128

When a user passes a non-string object into {{ log() }}, we end up passing that object straight through to scrub_secrets. Which expects to be receiving a string!

The type signature of log actually expects to be receiving a string, too:

https://github.com/dbt-labs/dbt-core/blob/6e8388c653c6bbeee934aea7db93730bd88df091/core/dbt/context/base.py#L543-L562

Resolution

We could put this logic anywhere, but it feels like the simplest and most appropriate fix may well be to stringify whatever users are passing into log, before firing it:

    def log(msg: str, info: bool = False) -> str:
        ...
        # user-provided msg should be a string
        msg = str(msg)
        if info:
            fire_event(MacroEventInfo(msg=msg))
        else:
            fire_event(MacroEventDebug(msg=msg))
        return ""

This successfully handles some pretty weird edge cases:

{{ log(this, info = true) }}
{{ log(create_table_as, info = true) }}
{{ log(context, info = true) }}
12:11:26  "jerco"."dbt_jcohen"."model_a"
12:11:26  <dbt.clients.jinja.MacroGenerator object at 0x10dca87f0>
... giant context blob ...

Still, we should make sure it raises a useful error, in case users try passing in objects that aren't string serializable. (I can't think of an example right now, but maybe there is one?)

jtcohen6 avatar Jun 16 '22 12:06 jtcohen6

@VersusFacit does this seem related to the ticket you just closed out? Should we solve this further down the stack? #5436 #5874

leahwicz avatar Sep 22 '22 14:09 leahwicz

Solution provided in this PR

VersusFacit avatar Oct 17 '22 01:10 VersusFacit