Skip to content

docs(commerce): add service README documenting LOG_LEVEL and env vars#7996

Open
mvanhorn wants to merge 2 commits intographql-hive:mainfrom
mvanhorn:osc/3462-commerce-readme
Open

docs(commerce): add service README documenting LOG_LEVEL and env vars#7996
mvanhorn wants to merge 2 commits intographql-hive:mainfrom
mvanhorn:osc/3462-commerce-readme

Conversation

@mvanhorn
Copy link
Copy Markdown
Contributor

Summary

Closes #3462. Adds a README for the @hive/commerce service, documenting every environment variable it parses (including LOG_LEVEL). The commerce service was the only one under packages/services/ without a README and was the only service still missing LOG_LEVEL documentation from the original ask.

Why this matters

@n1ru4l opened #3462 asking that LOG_LEVEL be documented in every service README. Investigating packages/services/*/src/environment.ts shows 7 services parse LOG_LEVEL today: commerce, policy, schema, server, tokens, usage, usage-ingestor, workflows. Cross-referencing each against its directory's README.md, 6 of 7 already document LOG_LEVEL — only commerce was missing, because it had no README at all.

The new README also gives adopters a single place to look up what commerce is (a tRPC router covering usageEstimator, rateLimit, and stripeBilling, per src/api.ts), along with its default port 4012 (from environment.ts:64), which sibling READMEs already reference via the COMMERCE_ENDPOINT variable.

Changes

  • New file: packages/services/commerce/README.md
    • Header paragraph describing the tRPC router surface and default port.
    • Configuration table listing every variable defined in packages/services/commerce/src/environment.ts, using the same GitHub-flavored Markdown layout as packages/services/tokens/README.md and packages/services/usage/README.md.
    • Required column mirrors the zod schema (non-optional fields = Yes).
    • LOG_LEVEL description copied verbatim from the voice used in the sibling READMEs.

No other files changed.

Testing

  • Ran the same enumeration that identified the gap:
    • grep -rln "LOG_LEVEL" packages/services/*/src/environment.ts returns the 7 services above.
    • ls packages/services/commerce/ before this PR: LICENSE package.json src tsconfig.json turbo.json (no README).
  • Every env var in the new table was read directly from environment.ts (no memory, no guesses). Default values and optionality flags match the code.
  • The COMMERCE_ENDPOINT cross-reference (http://127.0.0.1:4012) already lives in packages/services/server/README.md:96 and packages/services/usage/README.md:14, so the port claim is cross-verified.

Closes #3462

This contribution was developed with AI assistance (Claude Code).

Every service that parses `LOG_LEVEL` should document it in its README
per graphql-hive#3462. The `commerce` service was the only one missing both a README
and `LOG_LEVEL` documentation.

Closes graphql-hive#3462
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds a comprehensive README for the @hive/commerce service, documenting its purpose and configuration options. The review feedback focuses on improving the accuracy and consistency of the configuration table by explicitly documenting default values for the RELEASE, PROMETHEUS_METRICS_LABEL_INSTANCE, REQUEST_LOGGING, and LOG_LEVEL environment variables.

Comment thread packages/services/commerce/README.md Outdated
| ----------------------------------- | -------- | -------------------------------------------------------------------------------------------------------- | ---------------------------------------------------- |
| `PORT` | No | The port this service is running on. | `4012` |
| `ENVIRONMENT` | No | The environment of your Hive app. (**Note:** This will be used for Sentry reporting.) | `staging` |
| `RELEASE` | No | The release identifier reported with Sentry events. | `v9.6.0` |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The RELEASE variable defaults to local in the code (packages/services/commerce/src/environment.ts:141). It should be documented here for completeness and consistency with other variables in this table.

Suggested change
| `RELEASE` | No | The release identifier reported with Sentry events. | `v9.6.0` |
| `RELEASE` | No | The release identifier reported with Sentry events. Defaults to local. | `v9.6.0` |
References
  1. Focus on correctness and providing accurate documentation for the service configuration. (link)

Comment thread packages/services/commerce/README.md Outdated
| `SENTRY` | No | Whether Sentry error reporting should be enabled. | `1` (enabled) or `0` (disabled) |
| `SENTRY_DSN` | No | The DSN for reporting errors to Sentry. Required when `SENTRY=1`. | `https://dooobars@o557896.ingest.sentry.io/12121212` |
| `PROMETHEUS_METRICS` | No | Whether Prometheus metrics should be enabled. | `1` (enabled) or `0` (disabled) |
| `PROMETHEUS_METRICS_LABEL_INSTANCE` | No | The instance label added for the prometheus metrics. | `commerce` |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The code defaults this to rate-limit (packages/services/commerce/src/environment.ts:173). While this might be a copy-paste artifact in the code (given this is the commerce service), the documentation should reflect the actual behavior.

Suggested change
| `PROMETHEUS_METRICS_LABEL_INSTANCE` | No | The instance label added for the prometheus metrics. | `commerce` |
| `PROMETHEUS_METRICS_LABEL_INSTANCE` | No | The instance label added for the prometheus metrics. Defaults to rate-limit. | `commerce` |
References
  1. Focus on correctness and providing accurate documentation for the service configuration. (link)

Comment thread packages/services/commerce/README.md Outdated
| `PROMETHEUS_METRICS` | No | Whether Prometheus metrics should be enabled. | `1` (enabled) or `0` (disabled) |
| `PROMETHEUS_METRICS_LABEL_INSTANCE` | No | The instance label added for the prometheus metrics. | `commerce` |
| `PROMETHEUS_METRICS_PORT` | No | Port on which prometheus metrics are exposed. Defaults to `10254`. | `10254` |
| `REQUEST_LOGGING` | No | Log http requests. | `1` (enabled) or `0` (disabled) |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The REQUEST_LOGGING variable defaults to 1 (enabled) in the code (packages/services/commerce/src/environment.ts:59-61). This should be mentioned in the description to match the format used for other variables in this file.

Suggested change
| `REQUEST_LOGGING` | No | Log http requests. | `1` (enabled) or `0` (disabled) |
| `REQUEST_LOGGING` | No | Log http requests. Defaults to 1. | `1` (enabled) or `0` (disabled) |
References
  1. Focus on correctness and providing accurate documentation for the service configuration. (link)

Comment thread packages/services/commerce/README.md Outdated
| `PROMETHEUS_METRICS_LABEL_INSTANCE` | No | The instance label added for the prometheus metrics. | `commerce` |
| `PROMETHEUS_METRICS_PORT` | No | Port on which prometheus metrics are exposed. Defaults to `10254`. | `10254` |
| `REQUEST_LOGGING` | No | Log http requests. | `1` (enabled) or `0` (disabled) |
| `LOG_LEVEL` | No | The verbosity of the service logs. One of `trace`, `debug`, `info`, `warn`, `error`, `fatal` or `silent` | `info` (default) |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

For consistency with other entries in this table (e.g., lines 28, 30, 35), the default value should be mentioned in the 'Description' column rather than the 'Example Value' column.

Suggested change
| `LOG_LEVEL` | No | The verbosity of the service logs. One of `trace`, `debug`, `info`, `warn`, `error`, `fatal` or `silent` | `info` (default) |
| `LOG_LEVEL` | No | The verbosity of the service logs. One of `trace`, `debug`, `info`, `warn`, `error`, `fatal` or `silent`. Defaults to info. | `info` |
References
  1. Focus on correctness and providing accurate documentation for the service configuration. (link)

…CS_LABEL_INSTANCE, REQUEST_LOGGING, and LOG_LEVEL

gemini-code-assist review on graphql-hive#7996 asked for each env var's default to
appear in the Description column for consistency with the rest of the
table. Verified against src/environment.ts:

- RELEASE -> 'local' (line 141)
- PROMETHEUS_METRICS_LABEL_INSTANCE -> 'rate-limit' (line 173; likely
  a copy-paste from the rate-limit service, but it's the current
  behavior)
- REQUEST_LOGGING -> '1' (line 59-61 .default('1'))
- LOG_LEVEL -> 'info' (line 166)
@mvanhorn
Copy link
Copy Markdown
Contributor Author

Added defaults in af81781. Verified each against packages/services/commerce/src/environment.ts:

  • RELEASE -> 'local' (line 141)
  • PROMETHEUS_METRICS_LABEL_INSTANCE -> 'rate-limit' (line 173 - noted separately: looks like a copy-paste from the rate-limit service, worth a follow-up to change to 'commerce' or lift to config. Kept the doc matching current behavior.)
  • REQUEST_LOGGING -> '1' (line 59 .default('1'))
  • LOG_LEVEL -> 'info' (line 166). Moved the info default from the Example column into the Description to match how other rows in the table call out defaults.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

document LOG_LEVEL environment variable for services in README

1 participant