Feature/api region configuration#22
Feature/api region configuration#22mueller-thomas-vse wants to merge 2 commits intocloudbring:masterfrom
Conversation
- Add NEW_RELIC_API_REGION environment variable (us/eu) - Support both US (default) and EU API endpoints - US endpoint: https://api.newrelic.com/graphql - EU endpoint: https://api.eu.newrelic.com/graphql - Updated .env.example with new configuration option - Added comprehensive test suite with 8 test cases - All 162 tests passing with >90% coverage
WalkthroughThis pull request introduces New Relic API region configuration support. A new environment variable Changes
Sequence DiagramsequenceDiagram
participant User
participant NewRelicClient as NewRelicClient
participant Env as Environment
participant NerdGraph as NerdGraph API
User->>NewRelicClient: new NewRelicClient()
NewRelicClient->>Env: read NEW_RELIC_API_REGION
alt Region = "eu"
Env-->>NewRelicClient: "eu"
NewRelicClient->>NewRelicClient: set nerdGraphUrl = eu endpoint
else Region = "us" or default
Env-->>NewRelicClient: "us" or undefined
NewRelicClient->>NewRelicClient: set nerdGraphUrl = us endpoint
end
User->>NewRelicClient: executeQuery()
NewRelicClient->>NerdGraph: fetch(this.nerdGraphUrl, ...)
NerdGraph-->>NewRelicClient: response
NewRelicClient-->>User: result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/client/newrelic-client.ts (1)
9-15: Consider logging a warning for invalid region values.The implementation correctly defaults to the US endpoint for invalid region values, which provides a safe fallback. However, this silent fallback could make debugging configuration issues difficult.
Consider adding a warning log when an invalid region is provided:
function getNerdGraphUrl(): string { const region = (process.env.NEW_RELIC_API_REGION || 'us').toLowerCase(); if (region === 'eu') { return NERDGRAPH_URLS.eu; } + if (region !== 'us' && process.env.NEW_RELIC_API_REGION) { + console.warn(`Invalid NEW_RELIC_API_REGION "${process.env.NEW_RELIC_API_REGION}". Defaulting to US region.`); + } return NERDGRAPH_URLS.us; }Note: This is optional and depends on your logging strategy. The current behavior is acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.env.example(1 hunks)README.md(1 hunks)src/client/newrelic-client.ts(3 hunks)test/client/newrelic-client/api-region.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test/client/newrelic-client/api-region.test.ts (1)
src/client/newrelic-client.ts (1)
NewRelicClient(45-288)
🔇 Additional comments (9)
README.md (1)
212-217: LGTM! Clear and comprehensive documentation.The documentation clearly explains the new optional environment variable, its default value, and the mapping between region codes and GraphQL endpoints. This aligns perfectly with the implementation.
.env.example (1)
13-17: LGTM! Helpful configuration example.The environment variable example is well-documented with clear comments explaining the valid values and their corresponding endpoints. The default value is appropriately shown.
src/client/newrelic-client.ts (3)
1-4: LGTM! Clean constant definition.The
NERDGRAPH_URLSconstant is well-structured with proper TypeScript typing usingas constfor immutability and type narrowing.
48-48: LGTM! Appropriate use of instance property.Storing the resolved NerdGraph URL as an instance property is the right approach—it's determined once during construction and remains constant for the lifetime of the client instance.
Also applies to: 53-53
270-270: LGTM! Clean refactor to use configurable endpoint.The fetch call now correctly uses the instance property instead of a hardcoded URL, enabling region-aware API calls while maintaining backward compatibility.
test/client/newrelic-client/api-region.test.ts (4)
1-21: LGTM! Proper test setup and teardown.The test configuration correctly:
- Preserves and restores original
fetchandprocess.env- Clears mocks between tests to prevent interference
- Uses proper lifecycle hooks for isolation
23-114: LGTM! Comprehensive coverage of URL selection logic.This test suite thoroughly validates the
getNerdGraphUrlbehavior:
- Default US endpoint when no region is specified
- Explicit US and EU region selection
- Case-insensitive handling
- Safe fallback to US for invalid values
The assertions correctly verify that
fetchis called with the expected endpoint URL.
116-193: LGTM! Validates region-specific API operations.These tests confirm that the region configuration correctly flows through to actual API operations:
- NRQL queries use the EU endpoint when configured
- APM application listing uses the US endpoint when configured
- Mock responses are properly structured
- Results are validated for correctness
195-216: LGTM! Confirms region persistence.This test validates that the region is determined once during client construction and consistently used across multiple API calls, which is the correct behavior for this design.
|
#23 Was merged that has |
Pull Request: Add Configurable New Relic API Region Support
Branch:
feature/api-region-configurationDescription
This PR adds support for configurable New Relic API regions via the
NEW_RELIC_API_REGIONenvironment variable. Users can now select between US and EU API endpoints.Changes
✨ Features:
NEW_RELIC_API_REGIONenvironment variable supporthttps://api.newrelic.com/graphqlhttps://api.eu.newrelic.com/graphql📝 Documentation:
.env.examplewith new configuration optionREADME.mdwith region configuration details🧪 Testing:
api-region.test.ts) with 8 test cases:Test Results
✅ All Tests Passing:
Code Quality
✅ Code Style Checks:
Files Changed
src/client/newrelic-client.tsNERDGRAPH_URLwith configurable endpoint selectiongetNerdGraphUrl()function for dynamic URL resolutionnerdGraphUrlproperty toNewRelicClientclass.env.exampleNEW_RELIC_API_REGIONdocumentationus,euREADME.mdNEW_RELIC_API_REGIONwith region-to-endpoint mappingtest/client/newrelic-client/api-region.test.ts(NEW)How to Use
Set US Region (default):
Set EU Region:
export NEW_RELIC_API_REGION=euIn Configuration File:
{ "env": { "NEW_RELIC_API_KEY": "your-api-key", "NEW_RELIC_ACCOUNT_ID": "your-account-id", "NEW_RELIC_API_REGION": "eu" } }Backward Compatibility
✅ Fully backward compatible:
Follows Contribution Guidelines
✅ TDD approach with comprehensive tests
✅ >90% test coverage maintained
✅ Code formatted and linted
✅ Documentation updated
✅ Conventional commits
✅ All automated checks passing
Summary by CodeRabbit
New Features
Documentation
Tests