Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Add support for logging the idempotency key #5620

Open
2 tasks done
ChrisHills463 opened this issue Nov 22, 2024 · 5 comments
Open
2 tasks done

Feature request: Add support for logging the idempotency key #5620

ChrisHills463 opened this issue Nov 22, 2024 · 5 comments
Labels
feature-request feature request triage Pending triage from maintainers

Comments

@ChrisHills463
Copy link

Use case

Sometimes there may be a bug in an application that calls an API, and to resolve the bug it is necessary to delete the cached response. At present, as there is no indication of the key, it is difficult to find when there are many records.

Solution/User Experience

I would like a hook that is called when a new idempotency record is created.

Alternative solutions

Right now I have to manually go through the records to find the right one.

Acknowledgment

@ChrisHills463 ChrisHills463 added feature-request feature request triage Pending triage from maintainers labels Nov 22, 2024
@ChrisHills463 ChrisHills463 changed the title Feature request: Add supoort for logging the idempotency key Feature request: Add support for logging the idempotency key Nov 22, 2024
@TonySherman
Copy link
Contributor

I would be interested in this feature as well. The idempotency utility has another use where you can use the idempotency store as a cache. If you have an "expensive" function call, storing the output in the idempotency store is a really nice way to cache the response but there is no way to evict the item before the TTL.

An alternative to a hook on storing the item (at least for my use case) would be allowing a completely custom key so we can evict items from the idempotency store from outside of the lambda if we need.

@walmsles
Copy link
Contributor

Hi there - I worked on the original code for the idempotency hook. I feel you have everything you need already, but do correct me if I am missing something or what there is does not meet your requirements.

Looking at the implementation the hook gets called following loading and processing of the idempotent record. This is a "hook" and not a "wrapper" so the behaviour is implemented as follows:

        if data_record.status == STATUS_CONSTANTS["EXPIRED"]:
            raise IdempotencyInconsistentStateError("save_inprogress and get_record return inconsistent results.")
        if data_record.status == STATUS_CONSTANTS["INPROGRESS"]:
            if data_record.in_progress_expiry_timestamp is not None and data_record.in_progress_expiry_timestamp < int(
                datetime.datetime.now().timestamp() * 1000,
            ):
                raise IdempotencyInconsistentStateError(
                    "item should have been expired in-progress because it already time-outed.",
                )
            raise IdempotencyAlreadyInProgressError(
                f"Execution already in progress with idempotency key: "
                f"{self.persistence_store.event_key_jmespath}={data_record.idempotency_key}",
            )

        response_dict = data_record.response_json_as_dict()
        serialized_response = self.output_serializer.from_dict(response_dict) if response_dict else None

        if self.config.response_hook:
            logger.debug("Response hook configured, invoking function")
            return self.config.response_hook(serialized_response, data_record)

        return serialized_response

So the response hook will only be called under the following circumstances:

data_record.status === STATUS_CONSTANTS["INPROGRESS"] - When new Idempotent record is created
data_record.status === STATUS_CONSTANTS["COMPLETE"] - When a cached result is returned

Since this is a hook the exception handling in handle_for_status() takes precedence and the hook will be skipped if exceptions are raised. The hook will then run only for valid status values as outlined above.

The IdempotentDataRecord:

   Parameters
        ----------
        idempotency_key: str
            hashed representation of the idempotent data
        status: str, optional
            status of the idempotent record
        expiry_timestamp: int, optional
            time before the record should expire, in seconds
        in_progress_expiry_timestamp: int, optional
            time before the record should expire while in the INPROGRESS state, in seconds
        payload_hash: str, optional
            hashed representation of payload
        response_data: str, optional
            response data from previous executions using the record
        """
        self.idempotency_key = idempotency_key
        self.payload_hash = payload_hash
        self.expiry_timestamp = expiry_timestamp
        self.in_progress_expiry_timestamp = in_progress_expiry_timestamp
        self._status = status
        self.response_data = response_data

@walmsles
Copy link
Contributor

you can use the idempotency store as a cache. If you have an "expensive" function call, storing the output in the idempotency store is a really nice way to cache the response but there is no way to evict the item before the TTL.

An alternative to a hook on storing the item (at least for my use case) would be allowing a completely custom key so we can evict items from the idempotency store from outside of the lambda if we need.

I am wondering whether idempotency is the right solution for this use-case? Its more a caching use-case and enabling key "munging" is dangerous due to the different ways Idempotency needs to be handled for function use-case vs Lambda Invoke use-case. I feel these use-cases and responsibilities are not something to group together.

You have the option to inherit your own IdempotencyHandler from the AWS Lambda powertools one and use it for a cache use-case. Not that I typed this I feel it will be useful for an RFC on a caching style utility which could be the core of idempotency with idempotency an inherited special case of caching - then we have something really clean and useful for all use-cases without munging up responsibilities - what do you think @TonySherman ?

@TonySherman
Copy link
Contributor

@walmsles I think you're absolutely right. The idempotency was a quick fix as a cache but might be kind of a square peg in a round hole situation. I was also chatting with @leandrodamascena a little more in detail about my use case. I think a cache utility based on the current idempotency core could be very powerful. Let me know if I can make a feature request or provide any more information that would help!

@walmsles
Copy link
Contributor

I think a cache utility based on the current idempotency core could be very powerful. Let me know if I can make a feature request or provide any more information that would help!

@TonySherman let's make this an RFC and see what other customers think so we can meet all the use cases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request feature request triage Pending triage from maintainers
Projects
Development

No branches or pull requests

3 participants