cwh icon indicating copy to clipboard operation
cwh copied to clipboard

Remove sequence token for PutLogEvents

Open a10waveracer opened this issue 2 years ago • 4 comments

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...) On January 4, 2023, Amazon announced that the sequence token is no longer required for PutLogEvents. This PR updates the library to reflect this new change and also adds functionality to emulate createGroup for log streams.

  • What is the current behavior? (You can also link to an open issue here) The refreshSequenceToken() function gets called somewhat regularly throughout the code.

  • What is the new behavior (if this is a feature change)? No more refreshSequenceToken() or associated calls!

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?) Based upon my initial testing, this does not cause a breaking change -- I was able to slide this in seamlessly without any issues in my environment. The AWS announcement notes that this change is available in all commercial regions which, I assume, means that ex. GovCloud doesn't have support for this? But the API Documentation doesn't reflect this limitation... 🤷 This does open up the possibility for users to leverage the new createStream constructor option to skip the describeLogGroups calls (which can cause issues similar to #113 and, frankly, is the reason I'm doing this in the first place).

  • Other information: This is also addressed in request #114.

a10waveracer avatar Feb 21 '23 23:02 a10waveracer

Just a quick comment - proposed PR would be a very nice addition, as it will be possible to omit calls to describeLogStreams, which by default limited to 5 reqs per second

A-Shevchenko avatar Mar 06 '23 15:03 A-Shevchenko

@a10waveracer Since this repository's owner hasn't been active recently, I've offered to fork and continue this repository in the meantime. This PR looks great, so I'd like to incorporate it into my fork: phpnexus/cwh.

If you recreated this PR against the above repository, I'd be happy to approve and merge it.

markinjapan avatar May 31 '23 01:05 markinjapan

@a10waveracer FYI I have removed the use of sequence tokens and added a constructor option $createStream in v3.1.0 of my fork: https://github.com/phpnexus/cwh/releases/tag/v3.1.0

markinjapan avatar Jun 22 '23 06:06 markinjapan

@markinjapan that's phenomenal -- thanks for the heads up!

a10waveracer avatar Jul 15 '23 18:07 a10waveracer