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.

Sign up for GitHub

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

Jump to bottom

feat(telemetry): add option to omit context propagation #2946

Merged
merged 10 commits into from
Dec 10, 2024
Merged

feat(telemetry): add option to omit context propagation #2946

merged 10 commits into from
Dec 10, 2024

Conversation

fgozdz
Copy link
Contributor

@fgozdz fgozdz commented Nov 29, 2024 •
edited
Loading

ref #2942

@fgozdz fgozdz requested a review from manast November 29, 2024 21:10
@roggervalf
Copy link
Collaborator

Wondering if an extra option can be passed at job level as for the case that was reportes, only repeatables jobs in some cases need this logic but we can keep the base logic for all the other job types

@manast
Copy link
Contributor

manast commented Dec 2, 2024

Wondering if an extra option can be passed at job level as for the case that was reportes, only repeatables jobs in some cases need this logic but we can keep the base logic for all the other job types

In this particular case it was not really repeatable jobs, they were simulating repetition using custom code. However the point is valid, maybe it is better to have it as a per job config, worth to think more about it.

@fgozdz fgozdz marked this pull request as draft December 4, 2024 13:42
@fgozdz
Copy link
Contributor Author

fgozdz commented Dec 4, 2024

I have changed it to pass the option entirely in bullmq, inside the job. For now only in methods regarding enhancement issue.

@fgozdz fgozdz requested a review from roggervalf December 4, 2024 13:45
@manast manast marked this pull request as ready for review December 7, 2024 10:01
fgozdz
fgozdz commented Dec 8, 2024
View reviewed changes
Copy link
Contributor Author

@fgozdz fgozdz left a comment

Choose a reason for hiding this comment

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

LGTM

manast
manast approved these changes Dec 10, 2024
View reviewed changes
src/classes/queue.ts Outdated
@@ -404,7 +404,10 @@ export class Queue<
...this.jobsOpts,
...job.opts,
jobId: job.opts?.jobId,
tm: span && srcPropagationMedatada,
tm:
Copy link
Contributor

Choose a reason for hiding this comment

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

I am confused, why is the option used here "tm" instead of "telemetryMetadata" as on the other functions above?

@manast
Copy link
Contributor

manast commented Dec 10, 2024

I will squash this PR as there were a few commits on this one...

@manast manast merged commit 6514c33 into master Dec 10, 2024
12 checks passed
@manast manast deleted the feat/telemetry-option-omit-context branch December 10, 2024 14:32
github-actions bot pushed a commit that referenced this pull request Dec 10, 2024
@semantic-release-bot
chore(release): 5.34.0 [skip ci]
fc8fac9 
# [5.34.0](v5.33.1...v5.34.0) (2024-12-10)

### Features

* **telemetry:** add option to omit context propagation on jobs ([#2946](#2946)) ([6514c33](6514c33))
alexandresoro pushed a commit to alexandresoro/ouca that referenced this pull request Dec 22, 2024
@alexandresoro
fix(deps): update dependency bullmq to v5.34.4 (#403)
c37f23c 
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [bullmq](https://bullmq.io/) ([source](https://github.com/taskforcesh/bullmq)) | dependencies | minor | [`5.32.0` -> `5.34.4`](https://renovatebot.com/diffs/npm/bullmq/5.32.0/5.34.4) |

---

### Release Notes

<details>
<summary>taskforcesh/bullmq (bullmq)</summary>

### [`v5.34.4`](https://github.com/taskforcesh/bullmq/releases/tag/v5.34.4)

[Compare Source](taskforcesh/bullmq@v5.34.3...v5.34.4)

##### Bug Fixes

-   **sandbox:** fix issue where job could stay in active forever ([#&#8203;2979](taskforcesh/bullmq#2979)) ([c0a6bcd](taskforcesh/bullmq@c0a6bcd))

### [`v5.34.3`](https://github.com/taskforcesh/bullmq/releases/tag/v5.34.3)

[Compare Source](taskforcesh/bullmq@v5.34.2...v5.34.3)

##### Bug Fixes

-   **sandboxed:** fix detecting special errors by sending default messages ([#&#8203;2967](taskforcesh/bullmq#2967)) fixes [#&#8203;2962](taskforcesh/bullmq#2962) ([52b0e34](taskforcesh/bullmq@52b0e34))

### [`v5.34.2`](https://github.com/taskforcesh/bullmq/releases/tag/v5.34.2)

[Compare Source](taskforcesh/bullmq@v5.34.1...v5.34.2)

##### Bug Fixes

-   **scripts:** make sure jobs fields are not empty before unpack ([4360572](taskforcesh/bullmq@4360572))

### [`v5.34.1`](https://github.com/taskforcesh/bullmq/releases/tag/v5.34.1)

[Compare Source](taskforcesh/bullmq@v5.34.0...v5.34.1)

##### Bug Fixes

-   guarantee every repeatable jobs are slotted ([9917df1](taskforcesh/bullmq@9917df1))
-   **job-scheduler:** avoid duplicated delayed jobs when repeatable jobs are retried ([af75315](taskforcesh/bullmq@af75315))

### [`v5.34.0`](https://github.com/taskforcesh/bullmq/releases/tag/v5.34.0)

[Compare Source](taskforcesh/bullmq@v5.33.1...v5.34.0)

##### Features

-   **telemetry:** add option to omit context propagation on jobs ([#&#8203;2946](taskforcesh/bullmq#2946)) ([6514c33](taskforcesh/bullmq@6514c33))

### [`v5.33.1`](https://github.com/taskforcesh/bullmq/releases/tag/v5.33.1)

[Compare Source](taskforcesh/bullmq@v5.33.0...v5.33.1)

##### Bug Fixes

-   **job-scheduler:** omit deduplication and debounce options from template options ([#&#8203;2960](taskforcesh/bullmq#2960)) ([b5fa6a3](taskforcesh/bullmq@b5fa6a3))

### [`v5.33.0`](https://github.com/taskforcesh/bullmq/releases/tag/v5.33.0)

[Compare Source](taskforcesh/bullmq@v5.32.0...v5.33.0)

##### Features

-   replace multi by lua scripts in moveToFailed ([#&#8203;2958](taskforcesh/bullmq#2958)) ([c19c914](taskforcesh/bullmq@c19c914))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS44MC4wIiwidXBkYXRlZEluVmVyIjoiMzkuODAuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19-->

Reviewed-on: https://git.tristess.app/alexandresoro/ouca/pulls/403
Reviewed-by: Alexandre Soro <[email protected]>
Co-authored-by: renovate <[email protected]>
Co-committed-by: renovate <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@manast manast manast approved these changes

@roggervalf roggervalf Awaiting requested review from roggervalf

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fgozdz @roggervalf @manast