go-pagerduty icon indicating copy to clipboard operation
go-pagerduty copied to clipboard

Feature Request: Implement function to truncate payload details before sending overly long events

Open jspaleta opened this issue 4 years ago • 6 comments

So now that pagerduty is going to enforce the 512KB size it would be fantastic if there was a way to have the golang client library truncate the details automatically.

The issue is right now, the details in the client library is an object, so not arbitrarily truncated, individuals using the client library will have to sanitize the length of every field in that json object, or delibrately construct an object that meets the requirements.

As a backstop and aid to help them identify the problem, this client library could test the length of the details and if over something reasonable like 256K would replace the details with a reasonable small details object indicating the original object was too long. That way overly long events don't bounce, but the details are stripped.

jspaleta avatar May 08 '20 22:05 jspaleta

@jspaleta do you know, does the Events API return an error if you try to submit a payload larger than 512KB? Or does it silently swallow it? If this returns an error, I'd be tempted to do the same and require that the caller truncate the payload themselves since I am not sure how we'd even figure out the proper way to truncate it.

theckman avatar Feb 07 '21 07:02 theckman

Hey,

I believe event api returns a status 400 and a json message with details as to the specific error. Multiple things trigger a 400 response. It looks like they may have added additional length limits on individual payload items as well as the limit on the event itself.

https://developer.pagerduty.com/docs/events-api-v2/trigger-events/ payload.summary has a 1024 character limit payload.dedup_key has a 255 character limit

passing the error and related json error message with details through for the caller to use on failure makes a lot of sense.

jspaleta avatar Feb 11 '21 01:02 jspaleta

@jspaleta thank you for your comment. We recently made two changes, #265 and then the final implementation in #272, to provide a way to better consume errors from the API. That includes the StatusCode and any error strings contained within.

Would you take a look at what we did in #272, and let me know if they seem to address your request/concern? If so I'll close this with a note that it will be addressed in v1.4.0.

theckman avatar Feb 20 '21 21:02 theckman

I'm working to clean up old issues that have either been addressed or don't feel like there's anything to do right now. I'm leaning towards this request being a no-op right now, while being open to discussing it further if it does seem like transparent truncation is the right path forward.

Would love to hear your (and others') thoughts on that idea. If I don't hear anything within a few days I'll probably just close it, asking that the topic be raised via a new issue if someone wants to bring it up again.

theckman avatar Sep 03 '21 20:09 theckman

yes... automatic truncation might be the wrong thing. How about this... extend the golang sdk with a verification function so the event payload size can be tested client side for all the explicit API size limits being enforced.

Benefit of an sdk verification function:

  1. This will allow me to take whatever remediation steps I need to take client side to avoid an oversized event.
  2. Verification function can be used in CI/CD testing for client code, without having to hit the pagerduty API.
  3. provided SDK becomes source of truth for payload limits instead of client code independently trying to keep up with enforced API limit changes.

Though if I'm going to take this further, what I really need is a way to ask the SDK a couple of questions:

  1. What are actively API enforced event length (this will change with sdk version if the api limits change).
  2. How much space is left in an event object before I ship it off to the API. That way I can do the contextually correct details truncation client side as I'm building the event and avoid any error condition.

If I can ask the sdk how much space is left in the event, as I build it.. that definitely solves my problem. I really want to avoid banging against the pagerduty API and returning an over length error, if I can programmatically avoid that situation.

jspaleta avatar Sep 07 '21 17:09 jspaleta

I wouldn't be against reviewing a PR with the intent to merge that adds a verification method, although the one challenge we're going to face is making sure it's up to date with how the PagerDuty API behaves. Being transparent, as an outside contributor I likely would be unaware of that changing, and because of specifics of this repo is configured it's hard for me to independently release a new version if that value were to change.

Considering that, is it a good idea for this SDK to become the source of truth for that instead of relying on the API itself?

Separately from that, it's not entirely clear to me how they measure the payload size. Is it 512KB of JSON? 512KB of data once parsed? The documentation is pretty sparse on that information.

Are you doing this in your codebase now by json.Marhsal() the V2Event.Payload and then checking the length? If so have you been able to observe anything that hints to the answer about how the measurements happen? If not I can probably raise a ticket to ask.

theckman avatar Sep 09 '21 00:09 theckman