Skip to content

feat: Add custom attribute for cookie header size#28890

Merged
rgraber merged 5 commits intomasterfrom
rsgraber/ARCHBOM-1910-record-cookie-header-size
Oct 1, 2021
Merged

feat: Add custom attribute for cookie header size#28890
rgraber merged 5 commits intomasterfrom
rsgraber/ARCHBOM-1910-record-cookie-header-size

Conversation

@rgraber
Copy link
Contributor

@rgraber rgraber commented Sep 30, 2021

Description

Report the total size of request cookie headers as a custom attribute to New Relic to provide observability into when cookie headers are reaching size limits. Calculated by getting the byte size of request.META['HTTP_COOKIE'], which is the raw string that eventually gets parsed into request.COOKIES (see https://github.com/django/django/blob/2fcafd169b5fcf4bb6711ca8aa4d59d80225ec7a/django/core/handlers/wsgi.py#L135)

The total size ends up being about ~100B more than just adding all the names and values in request.COOKIES, which makes sense considering the string representation has extra semi colons and quotation marks and other delineators.

Update description of CAPTURE_COOKIE_SIZE toggle to specify that it enables more detailed reporting than just the total size

Deadline

None

@rgraber rgraber changed the title <!-- ## #### Note: the Lilac master branch has been created. Please consider whether your change #### should also be applied to Lilac. If so, make another pull request against the #### open-release/lilac.master branch, or ping @nedbat for help or questions. ## feat: Add custom attribute for cookie header size Sep 30, 2021
@rgraber rgraber marked this pull request as ready for review September 30, 2021 16:18
"""

if request.META.get('HTTP_COOKIE', None):
set_custom_attribute('cookies.header.size', len(request.META['HTTP_COOKIE'].encode('utf-8')))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why the encode call? I guess that's measure bytes rather than characters?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep.

@edx-status-bot
Copy link

Your PR has finished running tests. There were no failures.

@rgraber rgraber merged commit bf025a9 into master Oct 1, 2021
@rgraber rgraber deleted the rsgraber/ARCHBOM-1910-record-cookie-header-size branch October 1, 2021 12:09
@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants