Skip to content

Adapt RUM server onboarding to include httpd #27435

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

Merged
merged 9 commits into from
Feb 20, 2025
Merged

Conversation

pablomartinezbernardo
Copy link
Contributor

@pablomartinezbernardo pablomartinezbernardo commented Feb 4, 2025

What does this PR do? What is the motivation?

  • Add instructions on how to onboard with injection in the server for RUM
  • Small changes in the already existing content for clarity

Merge instructions

Merge readiness:

  • Ready for merge

Merge queue is enabled in this repo. To have it automatically merged after it receives the required reviews, create the PR (from a branch that follows the <yourname>/description naming convention) and then add the following PR comment:

/merge

Additional notes

Copy link
Contributor

github-actions bot commented Feb 4, 2025

Preview links (active after the build_preview check completes)

Modified Files

@allspain
Copy link

allspain commented Feb 6, 2025

Does HTTPD support allowedTracingUrls and excludedActivityUrls? If so we should edit the NGINX line to include HTTPD

@allspain
Copy link

allspain commented Feb 6, 2025

There's a paragraph towards the bottom of the page that says:

Content compression by upstream server: If NGINX is acting as a proxy and the upstream server has content compression (like gzip, zstd, or Brotli) enabled, the module may not inject RUM. Ensure that content compression is disabled on the upstream server and configure NGINX to compress the content.

Should this be just NGINX or any of the webservers?

@rtrieu rtrieu added the editorial review Waiting on a more in-depth review label Feb 10, 2025
@rtrieu
Copy link
Contributor

rtrieu commented Feb 10, 2025

Created DOCS-10037 for more in-depth review.

@pablomartinezbernardo
Copy link
Contributor Author

There's a paragraph towards the bottom of the page that says:

Content compression by upstream server: If NGINX is acting as a proxy and the upstream server has content compression (like gzip, zstd, or Brotli) enabled, the module may not inject RUM. Ensure that content compression is disabled on the upstream server and configure NGINX to compress the content.

Should this be just NGINX or any of the webservers?

It applies to all because they can all be configured as reverse proxies. It also feels redundant with the section above that reads "If proxying compressed traffic...". WGDYT @rtrieu ?

@pablomartinezbernardo pablomartinezbernardo marked this pull request as ready for review February 11, 2025 12:38
Copy link
Contributor

@rtrieu rtrieu left a comment

Choose a reason for hiding this comment

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

Some minor feedback for your review. But otherwise looks good!

@rtrieu
Copy link
Contributor

rtrieu commented Feb 12, 2025

There's a paragraph towards the bottom of the page that says:
Content compression by upstream server: If NGINX is acting as a proxy and the upstream server has content compression (like gzip, zstd, or Brotli) enabled, the module may not inject RUM. Ensure that content compression is disabled on the upstream server and configure NGINX to compress the content.
Should this be just NGINX or any of the webservers?

It applies to all because they can all be configured as reverse proxies. It also feels redundant with the section above that reads "If proxying compressed traffic...". WGDYT @rtrieu ?

I think you are right that this is repetitive. I'll push a commit with a suggestion to reconcile these two.

Copy link
Contributor

@dmehala dmehala left a comment

Choose a reason for hiding this comment

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

Few suggestions. Thank you @rtrieu.

Co-authored-by: Aymeric <[email protected]>
Co-authored-by: Damien Mehala <[email protected]>
Applying minor suggestions
@rtrieu
Copy link
Contributor

rtrieu commented Feb 20, 2025

/merge

@dd-devflow
Copy link

dd-devflow bot commented Feb 20, 2025

View all feedbacks in Devflow UI.
2025-02-20 16:01:12 UTC ℹ️ Start processing command /merge


2025-02-20 16:01:21 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.


2025-02-20 16:18:14 UTC ℹ️ MergeQueue: merge request added to the queue

The median merge time in master is 7m.


2025-02-20 16:27:10 UTC ℹ️ MergeQueue: This merge request was merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial review Waiting on a more in-depth review mergequeue-status: done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants