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

[DOCS-10034] Update Browser SDK beforeSend callback function details #27763

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rtrieu
Copy link
Contributor

@rtrieu rtrieu commented Feb 21, 2025

What does this PR do? What is the motivation?

Updates the description for the beforeSend callback function with more details

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

@rtrieu rtrieu added the WORK IN PROGRESS No review needed, it's a wip ;) label Feb 21, 2025
@rtrieu rtrieu requested review from a team as code owners February 21, 2025 19:55
Copy link
Contributor

Preview links (active after the build_preview check completes)

Modified Files

@rtrieu rtrieu removed the WORK IN PROGRESS No review needed, it's a wip ;) label Feb 24, 2025
Copy link
Contributor

@cswatt cswatt left a comment

Choose a reason for hiding this comment

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

I'm afraid I've left some pretty annoying feedback here

function beforeSend(log,context)
```

The potential `context` values are:
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this function do with the context argument—why would a user set it, and what is the effect of setting it?

I'm guessing it's for if a user wanted to only apply the beforeSend function to a subset of logs that match a specific context. When they set context, the beforeSend function applies only to logs that match context. Whatever it is, we should explicitly state it.

Comment on lines +598 to +601
| Value | Use Case |
|-------|---------|
| `isAborted` | For network log events. |
| `handlingStack` | For network, console, and logger events - this value shows where the log event was handled, which is useful for micro-frontends. |
Copy link
Contributor

Choose a reason for hiding this comment

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

What does isAborted indicate? For handlingStack, what does "where" mean—a location? Also, we should probably specify data type. For example (and these are just my guesses at filling in missing information):

Suggested change
| Value | Use Case |
|-------|---------|
| `isAborted` | For network log events. |
| `handlingStack` | For network, console, and logger events - this value shows where the log event was handled, which is useful for micro-frontends. |
| Value | Data Type | Meaning | Use Case |
|-------|---------|
| `isAborted` | Boolean | Whether the log event is aborted. | For network log events. |
| `handlingStack` | String | Where the log event was handled, which is useful for micro-frontends. For example... <insert example values> | For network, console, and logger events. |

It doesn't need to be in this exact format

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.

3 participants