Skip to content

Add diagnostics-semconv to standardize some attributes and headers #579

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daniyelnnr
Copy link
Contributor

What is the purpose of this pull request?

This pull request introduces a major refactor to centralize HTTP header constants into a new HeaderKeys object within the src/constants.ts file. This change improves maintainability and reduces redundancy by replacing individual header constants with a single structured object. Additionally, it introduces a new AttributeKeys object for semantic attributes and updates the logger to use these attributes.

Dependency Additions
  • Added a new dependency, @vtex/diagnostics-semconv, to support semantic conventions for diagnostics and tracing attributes.
Logger Enhancements
  • Updated the Logger class to use the new AttributeKeys object for semantic attributes, such as VTEX_OPERATION_ID and VTEX_ACCOUNT_NAME, instead of plain object properties. This ensures better alignment with the diagnostics library.
Cleanup and Refactoring
  • Removed unused imports and constants that were replaced by the centralized HeaderKeys object, further simplifying the codebase.

What problem is this solving?

Introduces @vtex/diagnostics-semconv library to support VTEX semantic conventions, in a reduced scope with respect to telemetry signals and emphasizing the context of logs. Future updates will include updates to the instrumentation using the diagnostics library, as well as the use of the semantic conventions of these signals.

How should this be manually tested?

The changes do not include new features or major behavior changes, so to test the changes simply build from this branch.

Screenshots or example usage

Types of changes

  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Requires change to documentation, which has been updated accordingly.

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request refactors the codebase to centralize HTTP header constants by replacing individual header constant imports with a unified HeaderKeys object, while also introducing an AttributeKeys object for semantic diagnostics. It updates various middleware, router, and logger files to use these new keys and removes the deprecated constant imports.

  • Consolidate header constants into HeaderKeys and update usage across the codebase.
  • Introduce AttributeKeys for diagnostic semantic attributes and update the Logger accordingly.
  • Remove unused imports and clean up legacy constants.

Reviewed Changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/service/worker/runtime/utils/recorder.ts Updated header constant usage to HeaderKeys.
src/service/worker/runtime/utils/context.ts Replaced legacy header keys with HeaderKeys in context setup.
src/service/worker/runtime/http/router.ts Updated route header constant to HeaderKeys.
src/service/worker/runtime/http/middlewares/vary.ts Replaced vary header constants with HeaderKeys.
src/service/worker/runtime/http/middlewares/context.ts Updated header references to use HeaderKeys in event context.
src/service/worker/runtime/http/middlewares/authTokens.ts Removed deprecated header import.
src/service/worker/runtime/graphql/middlewares/updateSchema.ts Updated header constant for provider to HeaderKeys.
src/service/worker/runtime/graphql/middlewares/response.ts Updated header usage to HeaderKeys for cache control and meta headers.
src/service/worker/runtime/events/router.ts Replaced legacy header constant with HeaderKeys for event handlers.
src/service/worker/runtime/events/middlewares/context.ts Updated event context to use HeaderKeys.
src/service/worker/runtime/builtIn/middlewares.ts Updated route header constant to HeaderKeys.
src/service/tracing/tracingMiddlewares.ts Replaced legacy header keys with HeaderKeys during tracing.
src/service/logger/logger.ts Updated logger to use AttributeKeys for semantic attributes.
src/constants.ts Introduced HeaderKeys and AttributeKeys objects and removed legacy header constants.
src/clients/janus/Segment.ts Updated product header to HeaderKeys.PRODUCT.
src/HttpClient/middlewares/request/setupAxios/interceptors/tracing/spanSetup.ts Updated router cache header constant to HeaderKeys.ROUTER_CACHE.
src/HttpClient/middlewares/cache.ts Replaced legacy header references with HeaderKeys in cache middleware.
src/HttpClient/HttpClient.ts Updated HTTP header settings to use HeaderKeys.
Files not reviewed (1)
  • package.json: Language not supported

Base automatically changed from update/logs-instrumentation to master April 29, 2025 17:21
Copy link

@wisneycardeal wisneycardeal left a comment

Choose a reason for hiding this comment

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

LGTM

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

Successfully merging this pull request may close these issues.

2 participants