Limit API calls for aws_sfn_state_machine_execution_history and ignore ExecutionDoesNotExist errors

[pdecat]

In its current state, the aws_sfn_state_machine_execution_history table is unusable for us not matter what criteria are used in queries because it tries to load all execution events from all executions of all state machines, including expired executions, which results in errors, e.g.:

2023-04-06 10:21:11.816 UTC [ERROR] steampipe-plugin-aws.plugin: [ERROR] 1680776471843: aws_sfn_state_machine_execution_history.getRowDataForExecutionHistory: api_error="operation error SFN: GetExecutionHistory, https response error StatusCode: 400, RequestID: d118ab6d-033c-453c-abdc-6e52142cac74, ExecutionDoesNotExist: Execution Does Not Exist: 'arn:aws:states:eu-west-3:****:execution:mysfn:eb3a80d6-7efe-4900-8ae8-fce10c75e52d'"

This PR:

• ignores expired executions for which history is no longer available
• reduces AWS API calls made by aws_sfn_state_machine_execution_history by filtering on execution_arn

Ideally, the list hydrate function of the aws_sfn_state_machine_execution_history table should depend on the the list hydrate function of the aws_sfn_state_machine_execution table instead of the one of the aws_sfn_state_machine table, but ParentHydrate functions cannot be chained across more than two tables right now.

Slack thread: https://steampipe.slack.com/archives/C044P668806/p1680791390660499?thread_ts=1680791226.554809&cid=C044P668806

TODO:

• rebase on main once typo: aws_sfn_state_machine_* #1686 is merged
• support having execution_arn used in in queries with getListValues(), e.g.:

with constants (r, s_arn, e_arn) as (
    values (
            'eu-west-3',
            'arn:aws:states:eu-west-3:123456789012:stateMachine:mysfn',
            'arn:aws:states:eu-west-3:123456789012:execution:mysfn:map-%'
        )
)
select id,
    execution_arn,
    execution_failed_event_details
from aws_sfn_state_machine_execution_history,
    constants
where region = r
    and execution_arn in (
        select execution_arn
        from aws_sfn_state_machine_execution,
            constants
        where region = r
            and state_machine_arn = s_arn
            and execution_arn like e_arn
    )
    and execution_failed_event_details is not null;

Integration test logs

Logs

Add passing integration test logs here

Example query results

Results

Add example SQL query results here (please include the input queries as well)

[pdecat]

This PR needs more work before being complete.
Moved the first two commits into #1686 which should be mergeable faster.

[cbruno10]

@pdecat Thanks for opening this PR with such thoughtful changes and considerations! I've left a few questions and suggestions, can you please have a look?

[cbruno10]

Were there any functional differences with this change, or more stylistic changes?

[pdecat]

As it uses d.Quals instead of d.EqualsQuals, I believe it also makes queries not using = but in more performant (edit: on second thought, in queries are probably processed as several = queries):

select * from aws_sfn_state_machine_execution
where state_machine_arn in (
  select state_machine_arn
  from aws_sfn_state_machine
  where name = 'mystatemachine'
)

See

• https://github.com/turbot/steampipe-plugin-sdk/blob/2b7903e9b3d0aee3b456819a8896d1fd4d4c73ce/plugin/query_data.go#L393
• https://github.com/turbot/steampipe-plugin-sdk/blob/49556d376edd992a9ff0cb60485bd50c098604ac/plugin/key_column_qual_map.go#L22
• https://github.com/turbot/steampipe-plugin-sdk/blob/d4126181dfc96bcfc3014262326304ed9c2a1401/plugin/key_column_qual.go#L55
• https://github.com/turbot/steampipe-plugin-sdk/blob/49556d376edd992a9ff0cb60485bd50c098604ac/plugin/quals/qual.go#L58

On second thought, this may cause issues if other operators than = and in are used, e.g. like as the string value wouldn't match the checks done afterward. This is because getQualsValueByColumndoes not consider the operator at all forstring` type.

I will do more testing on that.

[cbruno10]

If we're given an execution_arn key qual, I wonder if this function even needs to call the ListExecutions API method?

Since we'd have a specific execution_arn when we enter this function, we could instead just call GetExecutionHistory once, passing in the given execution_arn and then returning the history data (or 0 rows if there is no history data).

If we don't get an execution_arn, then we would rely on listing all executions and then getting the histories for each of them (like the function does today).

This isn't normally how we'd want to do it, but due to the lack of nested parent hydrates, this may be a way to reduce the number of API calls we do.

@pdecat Does this approach seem like something that would work and cut down on total number of requests when execution_arn is passed in?

[pdecat]

Good point! I've implemented what you suggested, PTAL.

[cbruno10]

Hey @pdecat , I see a lot of commits have been pushed last week, just wanted to check in if you feel the PR is ready for review again? Or are there additional changes you're looking to make? Thanks!

[pdecat]

Hi @cbruno10, I indeed pushed some fixes to make it usable, but did not find time to fully test it.

One controversial change that would be worth reviewing anyway is my last commit where I disable parallelization of requests as the AWS API for retrieving history execution events heavily rate limits requests otherwise.

[misraved]

@pdecat is this PR ready for final review?

Excited to get the changes to the hub 👍.

[pdecat]

Hi @misraved ,

@pdecat is this PR ready for final review?

Sadly not 😕

Querying Step Functions execution history somehow works a bit better with this PR for me, but I still haven't managed execute my complex queries against our large dataset without hitting 429 rate limiting errors.
I've set aside work on this, so I marked it as draft for now.

Feel free to test or hack with it if you desire.

Things I figured that could help IMO are addressing turbot/steampipe-plugin-sdk#194 and turbot/steampipe-plugin-sdk#394

[karanpopat]

Hey @pdecat, I was testing through this table and found a small issue with ListConfig.
When passing a value to the key column execution_arn here, it returns duplicate rows. The number of duplicate rows I observed was equal to the count of aws_sfn_state_machine_execution resources.

I believe this could be because the API getting called multiple times using the same execution_arn while looping through each parent hydrate item.

[cbruno10]

Hey @pdecat , we're still interested in improving the aws_sfn_state_machine_execution_history table and discussing the approach more. This PR has been open for a while though, so we're going to close it, but happy to continue the discussion in it.

If/when you're ready for another review, feel free to re-open it, thanks!

[pdecat]

Hi @cbruno10, I'll probably revisit this once turbot/steampipe-plugin-sdk#618 is merged and released.