improvement: include context in inspect consistently

[felixbr]

Contributor checklist

• I accept the AI Policy, or AI was not used in the creation of this PR.
• Bug fixes include regression tests
• Chores
• Documentation changes
• Features include unit/acceptance tests
• Refactoring
• Update dependencies

Description

This came up when I was debugging (using dbg()) why my scope wasn't correctly propagated as context in my project. It took me a while to figure out that it was indeed correctly propagated but calling dbg(query) does not show whether or not there's a context or its contents.

My goal here is to save my future self and everyone else the time and frustration by putting the context into the Inspect implementation.

Design Decisions

When I asked Zack if I could simply add it he was cautious and said it had been intentionally left out due to concerns about leaking sensitive data. A very valid concern given that inspect is often used for logging.

My primary goal is to show that there is a (non empty) context present, not necessarily what its contents are. If the content is important you can look at it by directly calling dbg(query.context) in any case.

There is already a config setting called :show_sensitive? which is used by the Ash.Helpers.redact/1 function. I'm using this function if the context is non empty so it will show up as context: "**redacted**" by default, which gives enough of a clue that there is something present without potentially leaking something sensitive.

With the setting enabled it embeds the context as expected.

Current Implementation and Consistency

While writing this PR I looked at other entities which are passed to callbacks and carry the context inside and how they handle inspect (mostly Ash.Changeset and Ash.ActionInput).

As far as I can tell there is no consistent logic being applied. For example one implementation removes the :privat key from context but embeds it without redacting anything. The one in Ash.Query omits it fully and in Ash.ActionInput there's no custom Inspect so it seems to just show everything as it is.

I don't want to stray too far from my original goal but if you think it's worthwhile to have a consistent logic behind all of these structs, then we can discuss what the "spec" looks like and I'll try to implement that.

What I've tried to implement so far is this:
Structs passed to callbacks which contain a :context map should

• not include context in their Inspect output if the context is empty
• include context: "**redacted**" if the context is non empty if :show_sensitive? is false (default)
• include context: with all keys except :private embedded if :show_sensitive? is true
• use Ash.Helpers.redact/1 where applicable for the above

Sorry for the wall of text and thanks for reading it anyway! 😄

References

• Related problem and Zack's response in Discord

[zachdaniel]

There will pretty much always be a context. Could we perhaps to some work to check which keys are commonly included in the context (mostly just private) and only show it if it has any keys aside from those?

[zachdaniel]

Perhaps only show the keys of the context? Context<keys: [:foo, :bar]>?

[felixbr]

Not sure how I would know ahead of time which keys are considered "common" and which are considered "additional" (aside from :private).

But showing the keys without values sounds like a great idea. I'll rework it soon™, possibly with a redact_context/redact_struct function so it's easier to have consistent behaviour 👍

Do you have any opinion if :private should be omitted from the output? When showing the full struct it makes sense to hide that internal implementation detail, but then again this is about debugging where internal state might be something a dev wants to know about.

[zachdaniel]

:private should be yeah. Let's just set it up with that list, and make the list be :private, i.e Map.keys(context) -- @hide_keys etc. And if I spot more later then we can add them to that list 😆