Add support for setting redacted keys via global

[roborourke]

This solves the issue of the filter not being available for the first trace sent which happens prior to WP bootstrapping.

Usage:

// In .config/load-early.php
$xray_redact_keys = [ '$_POST' => [ 'key1', 'key2' ] ];

Relates to #80

[ivankristianto]

@roborourke Though this works, but I feel there is potential downside with the use of global variable here.
For example, it could be overridden accidentally by other code.
I'm thinking to use constant here or composer env instead? So other engineers forced to add their key from one place.

[roborourke]

A constant would have the issue of preventing other code adding to it. It would be up to application developers to check for existing keys etc... This is a pattern pretty common in WP like $batcache for instance. It isn't ideal but I think it's the best balance between flexibility, extensibility and ease of use.

If you wanted to add to this from a plugin for instance you'd use:

global $xray_redact_keys;

$xray_redact_keys = array_merge_recursive( $xray_redact_keys ?? [], [
  '$_POST' => [ '... my custom keys ...' ],
] );

This can be documented in the Altis docs.

[ivankristianto]

Good from me 👍🏼

[kovshenin]

If everything is sent so early in the request cycle, what's the use of the aws_xray.redact_metadata_keys filter anyway? If we're providing a filter to redact keys, it should work consistently. Could we remove $_POST and other superglobals from the initial trace, and only send them later when apply_filters is available?

[roborourke]

@kovshenin fair point, could remove it as part of this update? @joehoyle what was the goal with sending all the metadata with the early request too? Is it necessary?

[kovshenin]

But if we're going to remove it as part of this update, then this update is no longer necessary :) We no longer need an additional way to influence the redacting if we can use the existing filter reliably. If we do, however, need these as part of the initial X-Ray dump, then I'm fine with the global approach.

[roborourke]

@kovshenin depends if we consider this a bug, if we do need the metadata in the initial trace and it's going to be backported then it should retain the filter too.

[joehoyle]

The end trace can fail, so the idea is to send the data with the in_progress trace as a "get the data tracked as reliably as possible". I wonder if actually though we should look at just loading xray after the user's altis config has been loaded. I don't see much downside with doing that, as it's unlikely to impact request timing too much.

[rmccue]

I wonder if actually though we should look at just loading xray after the user's altis config has been loaded. I don't see much downside with doing that, as it's unlikely to impact request timing too much.

Presumably we're also starting the Excimer profile at that point though, so we'd miss flamegraph data for anything before that?

[joehoyle]

Yes, though the "distance" between when we currently load it vs after the config is very small I think. Hence I think it's a small tradeoff

[rmccue]

Presumably we can do something a bit more nuanced though: start Excimer as soon as possible, but defer the first push of metadata until we've loaded those early bits.