Skip to content

Conversation

@tassoevan
Copy link
Contributor

@tassoevan tassoevan commented Nov 25, 2025

Proposed changes (including videos or screenshots)

It normalizes ESLint configuration for @rocket.chat/tracing.

Issue(s)

ARCH-1894

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • Chores
    • Updated ESLint configuration for the tracing package
    • Reorganized and consolidated build scripts and development dependencies
    • Integrated OpenTelemetry tracing framework dependencies

✏️ Tip: You can customize this high-level summary in your review settings.

@tassoevan tassoevan added this to the 7.14.0 milestone Nov 25, 2025
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Nov 25, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is targeting the wrong base branch. It should target 7.14.0, but it targets 7.13.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Nov 25, 2025

⚠️ No Changeset found

Latest commit: defc5a5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 25, 2025

Walkthrough

Configuration updates to the tracing package: ESLint ignore pattern changed from recursive to non-recursive matching, package.json reorganized with OpenTelemetry dependencies added, devDependencies restored, scripts consolidated, and Volta configuration introduced.

Changes

Cohort / File(s) Summary
ESLint Configuration
packages/tracing/.eslintrc.json
Updated ignorePatterns from **/dist (recursive) to dist (non-recursive), altering which directories named dist are ignored by ESLint.
Package Manifest
packages/tracing/package.json
Reorganized and consolidated scripts block; added dependencies field with OpenTelemetry packages (@opentelemetry/api, @opentelemetry/exporter-trace-otlp-grpc, @opentelemetry/sdk-node); restored and organized devDependencies block; introduced Volta configuration extending root package.json.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Verify ESLint pattern change correctly targets only package-local dist directories
  • Confirm all OpenTelemetry dependencies are pinned to appropriate versions
  • Check that scripts configuration matches intended build/test/lint workflows
  • Validate Volta configuration path extends root package.json correctly

Possibly related PRs

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

  • ggazzo

Poem

🐰 Configuration hops in neat array,
ESLint patterns find their way,
OpenTelemetry takes its place,
Scripts reorganized with grace,
Volta extends from root so high—
Build config's ready to fly!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive The linked issue ARCH-1894 has no detailed coding requirements specified, making it impossible to validate whether code changes meet specific objectives. Review the linked issue ARCH-1894 to ensure it contains specific coding requirements and verify that all stated objectives are addressed by the code changes.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore(tracing): Review build configuration' accurately reflects the main changes, which involve updating ESLint configuration and reorganizing package.json build-related settings in the tracing package.
Out of Scope Changes check ✅ Passed All changes (ESLint configuration update and package.json reorganization) directly relate to the PR title's focus on reviewing build configuration for the tracing package.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/tracing-build-configuration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

📦 Docker Image Size Report

📈 Changes

Service Current Baseline Change Percent
sum of all images 1.2GiB 1.2GiB +12MiB
rocketchat 358MiB 346MiB +12MiB
omnichannel-transcript-service 132MiB 132MiB -380B
queue-worker-service 132MiB 132MiB -1.3KiB
ddp-streamer-service 127MiB 127MiB -184B
account-service 114MiB 114MiB -2.7KiB
stream-hub-service 111MiB 111MiB +197B
authorization-service 111MiB 111MiB -62B
presence-service 111MiB 111MiB -1.1KiB

📊 Historical Trend

---
config:
  theme: "dark"
  xyChart:
    width: 900
    height: 400
---
xychart
  title "Image Size Evolution by Service (Last 30 Days + This PR)"
  x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/25 19:41 (PR)"]
  y-axis "Size (GB)" 0 --> 0.5
  line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
  line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
  line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
  line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13]
  line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
  line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13]
  line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.35]
  line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Loading

Statistics (last 7 days):

  • 📊 Average: 1.5GiB
  • ⬇️ Minimum: 1.2GiB
  • ⬆️ Maximum: 1.6GiB
  • 🎯 Current PR: 1.2GiB
ℹ️ About this report

This report compares Docker image sizes from this build against the develop baseline.

  • Tag: pr-37609
  • Baseline: develop
  • Timestamp: 2025-11-25 19:41:39 UTC
  • Historical data points: 7

Updated: Tue, 25 Nov 2025 19:41:39 GMT

@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.83%. Comparing base (f5eedbe) to head (defc5a5).
⚠️ Report is 3 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop   #37609   +/-   ##
========================================
  Coverage    68.83%   68.83%           
========================================
  Files         3361     3361           
  Lines       114340   114340           
  Branches     20619    20619           
========================================
+ Hits         78706    78709    +3     
- Misses       33536    33537    +1     
+ Partials      2098     2094    -4     
Flag Coverage Δ
e2e 57.36% <ø> (+0.02%) ⬆️
e2e-api 42.21% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tassoevan tassoevan marked this pull request as ready for review November 25, 2025 20:41
@ggazzo ggazzo merged commit 46272fb into develop Nov 25, 2025
87 of 92 checks passed
@ggazzo ggazzo deleted the chore/tracing-build-configuration branch November 25, 2025 20:43
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between f5eedbe and defc5a5.

📒 Files selected for processing (2)
  • packages/tracing/.eslintrc.json (1 hunks)
  • packages/tracing/package.json (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
🔇 Additional comments (3)
packages/tracing/.eslintrc.json (1)

3-3: Verify the ESLint ignore pattern change aligns with package structure.

The glob pattern changed from **/dist (recursive, matches dist at any depth) to dist (non-recursive, matches only root-level dist). Ensure this narrower scope doesn't inadvertently exclude nested dist directories that should be ignored, especially given the lint scripts in package.json run from the package root with . as the target.

packages/tracing/package.json (2)

10-16: Scripts configuration looks good.

The build, dev, lint, lint:fix, and testunit scripts are appropriately configured with correct TypeScript compilation and ESLint scoping.


29-31: Volta configuration looks good.

Using extends to reference the root package.json is the correct pattern for maintaining consistent tooling versions across the monorepo.

Comment on lines 17 to +21
"dependencies": {
"@opentelemetry/api": "^1.9.0",
"@opentelemetry/exporter-trace-otlp-grpc": "^0.54.2",
"@opentelemetry/sdk-node": "^0.54.2"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

@opentelemetry/api 1.9.0 npm package version


<web_search_result>
Yes — @opentelemetry/api version 1.9.0 is published on npm (listed as the package's current/latest release). [1][2]

Sources:
[1] npm organization package listing (opentelemetry) — shows @opentelemetry/api published version 1.9.0.
[2] jsDelivr / package page — @opentelemetry/api version 1.9.0.
</web_search_result>


@opentelemetry/sdk-node 0.54.2 @opentelemetry/exporter-trace-otlp-grpc 0.54.2 compatibility


<web_search_result>
Short answer: Yes — @opentelemetry/sdk-node@0.54.2 and @opentelemetry/exporter-trace-otlp-grpc@0.54.2 are compatible and intended to be used together (OpenTelemetry JS packages with the same release line are published to work together). See the exporter docs and the SDK guidance that exporters must be installed alongside @opentelemetry/sdk-node. [1][2][3]

References

  • Exporter docs (exporter-trace-otlp-grpc). [1]
  • SDK Node docs (note that exporters are installed alongside @opentelemetry/sdk-node). [2]
  • npm package listing for 0.54.2 showing that 0.54.x packages were published as a matching set. [3]
    </web_search_result>

@opentelemetry/api 1.9.0 @opentelemetry/sdk-node 0.54.2 version compatibility


<web_search_result>
Short answer: No — @opentelemetry/api 1.9.0 is not compatible with @opentelemetry/sdk-node 0.54.2. The official compatibility matrix pairs experimental SDK versions 0.54.x with API ~1.27.x, while API 1.9.x maps to much older experimental SDKs (≈0.35.x). See the OpenTelemetry JS compatibility table and the @opentelemetry/api 1.9.0 package info. [1][2]

Sources:

  • OpenTelemetry JS package compatibility table. [1]
  • @opentelemetry/api 1.9.0 package entry. [2]

References:
[1] https://github.com/open-telemetry/opentelemetry-js — Package Version Compatibility.
[2] https://jsr.io/%40opentelemetry/api — @opentelemetry/api@1.9.0 package page.
</web_search_result>


Update OpenTelemetry API version for SDK compatibility.

The @opentelemetry/api version 1.9.0 is incompatible with @opentelemetry/sdk-node 0.54.2. According to the OpenTelemetry JS compatibility matrix, SDK 0.54.x requires API ~1.27.x; API 1.9.x corresponds to much older SDK versions (~0.35.x). Update @opentelemetry/api to ^1.27.0 or later to align with the 0.54.2 SDK and exporter versions.

	"dependencies": {
		"@opentelemetry/api": "^1.27.0",
		"@opentelemetry/exporter-trace-otlp-grpc": "^0.54.2",
		"@opentelemetry/sdk-node": "^0.54.2"
	},
🤖 Prompt for AI Agents
In packages/tracing/package.json around lines 17 to 21 the @opentelemetry/api
dependency is pinned to ^1.9.0 which is incompatible with
@opentelemetry/sdk-node ^0.54.2; update the @opentelemetry/api entry to ^1.27.0
(or a compatible >=1.27.x) to match the SDK/exporter versions, then run your
package manager (npm/yarn/pnpm) to update the lockfile and verify the project
builds/tests pass.

Comment on lines +22 to +28
"devDependencies": {
"@types/jest": "~30.0.0",
"eslint": "~8.45.0",
"jest": "~30.2.0",
"ts-jest": "~29.4.5",
"typescript": "~5.9.3"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Invalid devDependencies versions.

The specified versions appear to be invalid:

  • jest: ~30.2.0 — Jest 30.x does not exist; the latest stable version is 29.x
  • @types/jest: ~30.0.0 — @types/jest 30.x does not exist and should match the Jest version used
  • Other versions may also need verification for accuracy

Please verify and correct these versions. Run the following script to check what versions are actually available:

#!/bin/bash
# Description: Check available versions of dev dependencies

npm view jest versions --json | tail -20
npm view @types/jest versions --json | tail -20
npm view eslint versions --json | tail -20
npm view typescript versions --json | tail -20
npm view ts-jest versions --json | tail -20
🤖 Prompt for AI Agents
In packages/tracing/package.json around lines 22-28, the devDependencies list
contains invalid/incorrect versions (e.g., jest ~30.2.0 and @types/jest
~30.0.0). Verify the actual available stable versions (use npm view/npm info as
suggested), then update package.json to use a valid Jest 29.x version and the
matching @types/jest 29.x, ensure ts-jest and typescript versions are compatible
with that Jest release, save the corrected versions in package.json, run npm
install to update lockfile, and run the test suite to confirm everything works.

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.

4 participants