Metrics, tracing, logging, error tracking and instrumentation of production systems.
204 instructions from 7 repositories. Last updated 2025-09-03.
Always measure performance impact before making optimization decisions or skipping potential optimizations due to complexity assumptions. Performance characteristics can be counterintuitive, and assumptions about complexity vs. performance trade-offs should be validated with actual benchmarks.
Key practices:
Example from the codebase: Resource filters were initially skipped in trace operators due to complexity concerns, but after performance testing showed benefits, they were implemented. Similarly, regex matching was replaced with direct lookups after discovering it was 5x slower.
This approach prevents premature optimization while ensuring that beneficial optimizations aren’t overlooked due to unfounded complexity concerns.
All new code additions, regardless of complexity, should include corresponding unit tests to ensure functionality works as expected and to prevent regressions. This applies to new modules, utility functions, and any other code components.
When adding new functionality:
Example from discussions:
// For a new module like spanpercentile/translator.go
func TestBuildSpanPercentileQuery(t *testing.T) {
// Test the queries getting built
}
// For utility functions like ToNanoSecs
func TestToNanoSecs(t *testing.T) {
// Test seconds, milliseconds, microseconds, and ns to ns conversions
}
Even if the new code might be indirectly tested by consumers, dedicated unit tests provide better isolation, clearer failure diagnostics, and serve as documentation of expected behavior.
Use meaningful, descriptive names that clearly convey purpose and maintain consistency across similar concepts throughout the codebase. Avoid magic strings, cryptic abbreviations, and inconsistent naming patterns.
Key principles:
"clickhouse_max_threads", create const ClickhouseMaxThreadsKey = "clickhouse_max_threads"serverAddrLegacyIdx and serverAddrCurrentIdx instead of mixing serverAddrIdx and netPeerIdx)k, m with meaningful ones like metricName, isNormalizedGetTransitionedMetric (singular) instead of GetTransitionedMetrics when operating on single itemsNewNotFoundf instead of NotFoundNew for natural readability, and Start/End instead of StartDate/EndDate for timestamp fieldsalert_id consistently if other IDs follow underscore conventionExample of good naming consistency:
const (
ServerNameLegacyKey = "net.peer.name"
ServerNameCurrentKey = "server.address"
URLPathLegacyKey = "http.url"
URLPathCurrentKey = "url.full"
)
// Consistent variable naming
serverAddrLegacyIdx := -1
serverAddrCurrentIdx := -1
This approach improves code readability, reduces confusion during reviews, and makes the codebase more maintainable by establishing clear naming patterns that developers can follow consistently.
Maintain proper package organization by following established structural patterns and separation of concerns. Code should be organized into appropriate packages based on functionality and responsibility.
Key organizational principles:
pkg/modules/, not in integrations/ or other generic folderspkg/types/ with appropriate sub-packages (e.g., cachetypes, quickfiltertypes)New functions within the types packageExample of proper organization:
// pkg/types/tracefunnel/tracefunnel.go - Type definitions
type Funnel struct { ... }
// pkg/modules/tracefunnel/module.go - Interface declaration
type Module interface { ... }
// pkg/modules/tracefunnel/impltracefunnel/module.go - Implementation
type module struct { ... }
// pkg/modules/tracefunnel/tracefunneltest/mocks.go - Test mocks
type MockModule struct { ... }
This organization improves code maintainability, prevents circular dependencies, and makes the codebase more navigable for developers.
Replace hardcoded color values, spacing, and other design-related constants with design tokens or CSS custom properties. This promotes consistency across the application, makes theme changes easier to implement, and reduces code duplication.
For colors, use predefined design system tokens:
// Instead of:
border-right: 1px solid #1d212d;
color: var(--text-color-secondary);
// Use:
border-right: 1px solid var(--bg-slate-400);
background: var(--bg-slate-500);
For repeated values that don’t have existing tokens, declare local CSS variables to avoid repetition and improve maintainability. This ensures a consistent visual language and makes global design changes more manageable.
Follow standard REST API conventions for HTTP methods, status codes, endpoint structure, and response completeness. Use appropriate access controls and implement proper pagination patterns.
Key principles:
POST /api/v1/orgs/me/resource for creation, GET /api/v1/orgs/me/resource/:id for retrievalExample of proper REST endpoint structure:
// Good: Standard REST conventions
traceFunnelsRouter.HandleFunc("", am.ViewAccess(aH.Create)).Methods(http.MethodPost) // POST /api/v1/orgs/me/trace-funnels
traceFunnelsRouter.HandleFunc("", am.ViewAccess(aH.List)).Methods(http.MethodGet) // GET /api/v1/orgs/me/trace-funnels
traceFunnelsRouter.HandleFunc("/{id}", am.ViewAccess(aH.Get)).Methods(http.MethodGet) // GET /api/v1/orgs/me/trace-funnels/:id
traceFunnelsRouter.HandleFunc("/{id}", am.ViewAccess(aH.Update)).Methods(http.MethodPut) // PUT /api/v1/orgs/me/trace-funnels/:id
// Bad: Non-standard endpoints
router.HandleFunc("/api/v1/export", am.OpenAccess(ah.Export)).Methods(http.MethodGet) // Should use viewAccess
traceFunnelsRouter.HandleFunc("/new", am.ViewAccess(aH.New)).Methods(http.MethodPost) // Should be POST to base path
This ensures APIs are predictable, secure, and follow industry standards that developers expect.
Database migrations must be designed to run safely multiple times without causing errors or data corruption. This involves using structured migration APIs instead of raw SQL, implementing proper checks for existing state, and avoiding unnecessary complexity that could lead to failures.
Key practices:
NewDropColumn, NewAddColumn instead of executing raw SQL queries for better safety and maintainabilityIF NOT EXISTS clauses)Example of idempotent migration pattern:
func (migration *example) Up(ctx context.Context, db *bun.DB) error {
// Use structured API with safety checks
_, err := tx.NewCreateTable().
Model((*ExampleModel)(nil)).
IfNotExists(). // Idempotent check
Exec(ctx)
// Avoid unnecessary complexity - only pass required constraints
column := &sqlschema.Column{
Name: sqlschema.ColumnName("new_field"),
DataType: sqlschema.DataTypeText,
Nullable: true,
}
// Pass nil for constraints if none needed, don't extract unused ones
sqls := migration.sqlschema.Operator().AddColumn(table, nil, column, nil)
}
func (migration *example) Down(ctx context.Context, db *bun.DB) error {
// Simply return nil if down migrations aren't used
return nil
}
This approach ensures migrations can be run reliably across different environments and deployment scenarios without manual intervention or risk of failure.
Ensure all required fields are included in API payloads and handle special field cases properly to prevent runtime errors and data inconsistencies.
Missing fields in API transformations can cause bugs and unexpected behavior. Always validate that essential fields like aggregations, filter, and having are properly passed through data transformations. Additionally, handle special cases where certain fields require different treatment based on context.
For example, when transforming query data, ensure all necessary fields are preserved:
queryData.push({
...baseQuery,
filters: queryFromData.filters,
filter: queryFromData.filter, // Don't forget these
aggregations: queryFromData.aggregations, // Missing causes bugs
having: queryFromData.having,
});
When dealing with special fields, implement proper conditional logic:
const fieldObj: TelemetryFieldKey = {
name: fieldName,
fieldDataType: column?.fieldDataType ?? (column?.dataType as FieldDataType),
signal: column?.signal ?? undefined,
};
// Only add fieldContext if the field is NOT deprecated
if (!isDeprecated && fieldName !== 'name') {
fieldObj.fieldContext = column?.fieldContext ?? (column?.type as FieldContext);
}
Always consider how missing or incorrectly structured fields will affect downstream API consumers and implement comprehensive field validation before sending payloads.
Choose variable, method, and property names that clearly communicate their purpose and context to avoid confusion and misinterpretation. Names should be self-explanatory and specific enough that their meaning is unambiguous within their usage context.
Avoid generic or ambiguous names that could have different meanings depending on the context. Instead, use descriptive names that explain both what the identifier represents and how it’s intended to be used.
Consider future extensibility when naming - avoid names that are too specific to current implementation details that might change.
Examples of improvements:
HasRunOnExplorer → calledFromHandleRunQuery (explains the source/context of the call)hasEditPermission → isEditorOrAdmin (specifies the exact permission level)isTenMinutesTracesTimeRange → useFocusedTracesTimeRange (describes the behavior, not the specific time constraint)This practice reduces cognitive load during code reviews and maintenance, making the codebase more self-documenting and less prone to misunderstandings.
Prefer direct hook usage over prop drilling and carefully consider context provider placement to maintain clean component architecture. When custom hooks are available, use them directly in components rather than passing their values through props. For context providers, aim for global placement when possible, but be mindful of routing and component lifecycle implications.
Example of preferred approach:
// Instead of prop drilling useTraceActions results
function AttributeActions() {
const traceActions = useTraceActions(); // Direct hook usage
// ... component logic
}
// For context providers, prefer global placement when feasible
// In AppRoutes/index.tsx:
<PreferenceContextProvider>
<Router>
{/* All routes have access to context */}
</Router>
</PreferenceContextProvider>
This approach reduces unnecessary prop passing, improves component independence, and creates cleaner data flow patterns. However, always test context provider placement changes thoroughly to ensure they don’t introduce routing or rendering issues.
When configuration structures evolve, implement fallback mechanisms to handle both old and new formats gracefully. This ensures backward compatibility during transitions and prevents breaking changes.
Key practices:
Example from query configuration migration:
// Check new format first, then fallback to legacy
metricName: queryData.aggregation?.[0].metricName || queryData.aggregateAttribute?.key
Example from TTL configuration update:
// Handle unit conversion during configuration migration
setLogsCurrentTTLValues((prev) => ({
...prev,
logs_ttl_duration_hrs: logsTotalRetentionPeriod || -1,
default_ttl_days: logsTotalRetentionPeriod
? logsTotalRetentionPeriod / 24 // convert Hours to days
: prev.default_ttl_days
}));
This approach maintains system stability while allowing configuration schemas to evolve over time.
When setting up observability infrastructure in build systems, use proper dependency management and descriptive naming. Define service dependencies explicitly rather than embedding commands, and use specific names that clearly identify the observability tools being orchestrated.
For example, instead of creating a target with embedded commands:
devenv-up: ## Start the dev stack
@cd .devenv/docker/clickhouse; docker compose up -d
@cd .devenv/docker/otel-collector; docker compose up -d
Use Make dependencies with descriptive target names:
devenv-up: devenv-clickhouse devenv-signoz-otel-collector
devenv-signoz-otel-collector: ## Run signoz-otel-collector in devenv
@cd .devenv/docker/otel-collector; docker compose up -d
This approach makes the observability stack setup more maintainable, allows for selective service startup, and clearly communicates which specific observability tools are being used.
Move all inline styles to dedicated CSS/SCSS files to improve maintainability, consistency, and separation of concerns. Inline styles make components harder to maintain, reduce reusability, and violate the principle of separating presentation from logic.
Instead of using inline styles:
// ❌ Avoid
<div style={{
display: 'flex',
alignItems: 'center',
gap: '8px'
}}>
<span style={{ color: 'white', fontSize: '14px' }}>
Status
</span>
</div>
// ❌ Avoid
<Typography.Paragraph
style={{
color: 'var(--text-vanilla-300)',
fontStyle: 'italic'
}}
>
Create proper CSS classes:
// ✅ Preferred
.status-container {
display: flex;
align-items: center;
gap: 8px;
.status-text {
color: white;
font-size: 14px;
}
}
.login-prompt {
color: var(--text-vanilla-300);
font-style: italic;
}
// ✅ Preferred
<div className="status-container">
<span className="status-text">Status</span>
</div>
<Typography.Paragraph className="login-prompt">
This approach enables better theming, easier maintenance, improved performance through CSS caching, and cleaner component code that focuses on logic rather than presentation details.
Always use database transactions when performing operations that require atomicity or could create race conditions. This includes scenarios where you check for existence before creating/updating records, or when multiple related database operations must succeed or fail together.
Follow the standard transaction pattern:
BeginTx()Example:
func (store *store) Update(ctx context.Context, funnel *traceFunnels.StorableFunnel) error {
tx, err := store.sqlstore.BunDB().BeginTx(ctx, nil)
if err != nil {
return err
}
defer tx.Rollback()
// Check if funnel exists
exists, err := tx.NewSelect().
Model((*traceFunnels.StorableFunnel)(nil)).
Where("name = ? AND org_id = ? AND id != ?", funnel.Name, funnel.OrgID, funnel.ID).
Exists(ctx)
if err != nil {
return err
}
if exists {
return errors.NewAlreadyExistsf("funnel with name already exists")
}
// Update the funnel
_, err = tx.NewUpdate().
Model(funnel).
Where("id = ?", funnel.ID).
Exec(ctx)
if err != nil {
return err
}
return tx.Commit()
}
This prevents race conditions where another process could create a conflicting record between the existence check and the update operation.
Configuration changes should be intentional, well-documented, and implemented using clean, understandable patterns. Avoid hacky workarounds like counter-based state management when simpler alternatives exist.
When modifying configuration defaults or synchronization logic:
Example of good practice:
// Clear, intentional configuration change
latency_pointer: 'end', // Default to 'end' for all steps except the 1st step
// Prefer clean boolean-based synchronization
const [needsResync, setNeedsResync] = useState(false);
// Instead of hacky counter-based approach
const [reSync, setReSync] = useState(0);
This ensures configuration management remains maintainable and the intent behind changes is preserved for future developers.
Only inherit from ABC when you have abstract methods that subclasses must implement. Avoid using ABC inheritance for classes that don’t define any abstract methods, as this adds unnecessary complexity without providing the intended benefits of the abstract base class pattern.
When you do use ABC, design it properly by defining abstract methods that enforce a contract for subclasses. This creates a clear interface that subclasses must follow.
Example of unnecessary ABC usage:
from abc import ABC
class LogsResource(ABC): # Unnecessary - no abstract methods
def __init__(self, labels: dict[str, str]):
self.labels = labels
def np_arr(self) -> np.array:
return np.array([self.labels])
Better approach - either remove ABC or add abstract methods:
from abc import ABC, abstractmethod
class LogsResource(ABC):
def __init__(self, labels: dict[str, str]):
self.labels = labels
@abstractmethod
def np_arr(self) -> np.array:
pass
@abstractmethod
def db_method(self):
pass
This ensures that ABC inheritance serves its intended purpose of enforcing implementation contracts rather than adding unnecessary abstraction layers.
Avoid storing sensitive credentials like usernames and passwords in environment variables. Instead, use dedicated secrets management systems such as HashiCorp Vault, AWS Secrets Manager, Azure Key Vault, or your CI/CD platform’s built-in secrets management.
Environment variables can be easily exposed through process listings, logs, or configuration dumps, making them unsuitable for sensitive data. Secrets management systems provide better security through encryption at rest, access controls, audit logging, and automatic rotation capabilities.
Example of problematic pattern:
// Avoid: Reading sensitive credentials from environment variables
const username = process.env.SIGNOZ_E2E_USERNAME;
const password = process.env.SIGNOZ_E2E_PASSWORD;
Instead, integrate with your organization’s secrets management solution or use your CI/CD platform’s secure secrets storage for accessing sensitive credentials during automated processes.
Ensure variable, function, and type names are consistent, descriptive, and follow Go naming conventions. Names should clearly convey their purpose and maintain consistency across related components.
Key principles:
EnableTypeAndUnitLabels vs AddTypeAndUnitLabels)Examples:
// Bad: Generic and inconsistent
func validateAttributes(attrs []string) // actually modifies input
var DefaultRegistry = prometheus.NewRegistry() // in main.go, shouldn't be exported
func handleOtlp(...) // inconsistent casing of initialism
// Good: Descriptive and consistent
func sanitizeAttributes(attrs []string) // clearly indicates modification
var registry = prometheus.NewRegistry() // unexported, no misleading "Default"
func handleOTLP(...) // consistent initialism casing
func BlockMetas() []BlockMeta // follows Go getter convention
func parseGoDuration(d string) time.Duration // accurately describes return type
This approach reduces cognitive load, improves code maintainability, and ensures the codebase follows established Go idioms.
Write tests that are direct, focused, and free of unnecessary complexity. Remove redundant test fields, use direct assertions with expected values, and leverage existing fixtures instead of creating temporary content.
Key principles:
require.Equal) with expected output instead of calculating or checking properties within testsExample of simplification:
// Before: Complex test with unused fields
type test struct {
errType errorType
err error // unused
resCode int
msg string // unused
errTypeToStatusCode ErrorTypeToStatusCode
}
// After: Simplified test structure
type test struct {
errType errorType
resCode int
errTypeToStatusCode ErrorTypeToStatusCode
}
// Use direct values instead of variables
api.respondError(w, &apiError{tc.errType, errors.New("message")}, "test")
This approach makes tests more maintainable, easier to understand, and less prone to errors from unused or redundant code.
Ensure documentation for observability systems provides comprehensive information including all required permissions, complete configuration options, and clear warnings about operational impacts. Monitoring and metrics systems often have significant performance and security implications that users must understand to operate them safely.
Key requirements:
Example of good documentation:
# Promoting all resource attributes to labels, except the ignored ones.
# 'ignore_resource_attributes' can be used to promote all resource attributes if set to an empty list:
#
# otlp:
# ignore_resource_attributes: []
#
# Be aware that changes in attributes received by the OTLP endpoint may result in time series churn and lead to high memory usage by the Prometheus server.
This approach prevents operational issues, security gaps, and user confusion in production monitoring systems.
When destructuring properties from objects that might contain undefined values, use the nullish coalescing operator (??) to provide explicit default values at the point of assignment. This prevents undefined values from propagating through your code and eliminates the need for repeated null checks later.
Instead of destructuring and handling nulls separately:
const { showIP } = params;
// Later in code: showIP ?? true
Provide the default immediately:
const showIP = params.showIP ?? true;
This pattern ensures that variables always have defined values, making your code more predictable and reducing the likelihood of null reference errors. Apply this approach consistently when accessing potentially undefined object properties, API responses, or configuration values.
Tests should explicitly assert both positive and negative expectations to make test intentions clear and prevent regressions. Use the expect syntax to specify not only what should happen, but also what should NOT happen in your tests.
When testing scenarios that should produce warnings or info messages, use expect warn or expect info. Equally important, when testing scenarios that should NOT produce these annotations, use expect no_warn or expect no_info to make this expectation explicit.
This approach serves two purposes:
Example:
# Explicit about expecting a warning
eval instant at 20m irate(http_requests_histogram{path="/c"}[20m])
expect warn
{path="/c"} {{sum:0.01 count:0.01 counter_reset_hint:gauge}}
# Explicit about NOT expecting a warning
eval instant at 20m irate(http_requests_histogram{path="/f"}[20m])
expect no_warn
{path="/f"} {{schema:-53 sum:0.01 count:0.01 custom_values:[5 10] buckets:[0.01]}}
Remember: the old eval behavior always asserted no annotations, but the new behavior is more tolerant. Be explicit about your expectations to maintain test robustness.
Use feature flags to gate new functionality for controlled rollouts. This allows for quick enabling/disabling without code reverts when issues arise, and supports gradual adoption through percentage-based rollouts.
When implementing a new feature:
# Use a descriptive flag name for the specific functionality
register(
"taskworker.enable_compression.rollout",
default=0.0, # Start with 0% rollout
flags=FLAG_AUTOMATOR_MODIFIABLE, # Allow runtime changes
)
# In your code, use standard rollout utilities
if features.has("organizations:revoke-org-auth-on-slug-rename", organization):
# Feature-specific code here
# For percentage-based rollouts, use consistent helpers
record_timing_rollout = options.get("sentry.tasks.record.timing.rollout")
if in_random_rollout(record_timing_rollout):
# Percentage-based rollout code here
# Design for runtime configuration when possible
compression_level = options.get("spans.buffer.compression.level")
zstd_compressor = zstandard.ZstdCompressor(level=compression_level) if compression_level != -1 else None
This approach enables safer deployments by allowing features to be rolled out gradually or quickly disabled if issues arise without requiring code reverts.
Choose variable, parameter, field, and class names that clearly communicate their purpose and context. Favor specific, self-explanatory identifiers over generic or ambiguous ones, and ensure they follow Python’s snake_case convention.
When naming variables:
func, type_str, or dict1 that don’t explain their purposeBad:
def serialize_rpc_event(self, event, group_cache, additional_attributes):
attributeDict = {
attribute: event[attribute]
for attribute in additional_attributes
if attribute in event
}
def _assemble_preprod_artifact(assemble_task, project_id, org_id, checksum, chunks, func):
# func is too generic and unclear what type of function is expected
Good:
def serialize_rpc_event(self, event, group_cache, additional_attributes):
attribute_dict = {
attribute: event[attribute]
for attribute in additional_attributes
if attribute in event
}
def _assemble_preprod_artifact(assemble_task, project_id, org_id, checksum, chunks, callback):
# callback clearly indicates the expected function's purpose
For class naming, ensure the name reflects the primary purpose:
# Instead of MultiProducerManager for a class that mainly acts as a producer
class MultiProducer:
def produce(self):
# implementation
When related but distinct concepts exist, use clear differentiators in names:
# Instead of ambiguous:
data_source_id = data_packet.source_id
# Use more specific naming:
data_packet_source_id = data_packet.source_id # Avoids confusion with DataSource.id
Implement defensive null checking to prevent NoneType errors, KeyError, and IndexError exceptions. When accessing dictionaries, collections, or optional values, verify existence before use.
For dictionary access:
# Unsafe - may raise KeyError:
message = event["data"]["payload"]["message"]
# Safe - uses default value if key missing:
message = event["data"]["payload"].get("message")
# With explicit default:
message = event["data"]["payload"].get("message", "Unknown")
For collections:
# Unsafe - may raise IndexError:
first_item = my_list[0]
# Safe - check before access:
if isinstance(my_list, list) and my_list:
first_item = my_list[0]
For optional attributes:
# Unsafe - may raise AttributeError or work unexpectedly:
if self.project_ids: # This passes if project_ids is empty list but fails if None
result = process_data(self.project_ids)
# Safe - explicit type check:
if self.project_ids is not None:
result = process_data(self.project_ids)
For inconsistent external data:
# Defensive type handling:
timestamp_raw = event.get("timestamp_ms", 0)
if isinstance(timestamp_raw, str):
try:
dt = datetime.fromisoformat(timestamp_raw.replace("Z", "+00:00"))
timestamp = dt.timestamp() * 1000 # Convert to milliseconds
except (ValueError, AttributeError):
timestamp = 0.0
else:
timestamp = float(timestamp_raw)
Algorithms must explicitly define and consistently handle special values like NaN, infinity, and edge cases across all mathematical operations. Special values can significantly impact computational results and should not be treated as afterthoughts.
Key requirements:
Example from PromQL aggregation functions:
// Good: Explicit NaN handling documentation
`quantile(phi, v)` calculates the φ-quantile, with NaN considered
the smallest possible value for ranking purposes.
// Good: Special case documentation
Special case for native histograms: NaN observations are considered
outside of any buckets. `histogram_fraction(-Inf, +Inf, b)` may
therefore be less than 1.
This prevents unexpected behavior and ensures algorithmic correctness, especially in statistical and mathematical computations where special values have well-defined meanings.
When working with feature flags and configuration conditions, keep your logic clean and up-to-date:
Simplify conditionals when flag states change: When a feature flag becomes permanently enabled or disabled, update and simplify all conditionals that reference it.
Access feature flags directly: Prefer checking feature flags directly rather than through abstraction layers that may become deprecated.
Track feature state changes accurately: When determining if a feature was newly activated, compare current state with previous state rather than relying only on current selections.
Example refactoring when a feature flag is now always true:
// Before: Complex condition with useEap feature flag
enabled: Boolean(webVital) && (useEap || isInp || (isSpansWebVital && useSpansWebVitals))
// After: Simplified condition since useEap is always true
enabled: Boolean(webVital) && (isInp || (isSpansWebVital && useSpansWebVitals))
Example for checking feature flags directly:
// Avoid using abstraction components for feature checking
<Feature features="organizations:user-feedback-ai-summaries">
{/* Component content */}
</Feature>
// Prefer direct access to feature flags
{organization.features.includes('organizations:user-feedback-ai-summaries') && (
/* Component content */
)}
Maintain consistent naming and symbol choices throughout code and documentation, prioritizing readability and avoiding ambiguity. When choosing between equivalent representations (like φ vs phi), select the more widely understood option and use it consistently across all related contexts.
Special characters or symbols that may not be familiar to all readers should be avoided in favor of their more accessible alternatives. This ensures that parameter names, function signatures, and documentation remain clear and understandable to developers with varying backgrounds.
For example, instead of mixing φ and phi in function documentation:
// Inconsistent - confusing for readers
quantile(φ, v) // in one place
quantile(phi, v) // in another place
// Consistent - clear for all readers
quantile(phi, v) // used everywhere
This principle applies to variable names, function parameters, mathematical symbols in documentation, and any identifier that has multiple valid representations.
Always ensure documentation and comments accurately reflect the actual code implementation. When modifying code, update all related documentation to maintain consistency. This applies to:
// If no fromStartTime or startTime is provided, use a 1-day lookback for a faster query // Be specific ```
// Process at most `max` jobs per 120 seconds // Not "30 seconds" if the value is 120_000
Accurate documentation reduces confusion, speeds up onboarding for new team members, and prevents bugs caused by misunderstanding code behavior. Review all documentation changes as carefully as you review code changes.
Follow the DRY (Don’t Repeat Yourself) principle by extracting repeated code patterns into helper functions, constants, or shared utilities. Code duplication leads to maintainability issues, inconsistencies, and bugs when one instance is updated but others are overlooked.
Examples of code that should be extracted:
// Before: Duplicated URL endpoint replacement logic
if (this.externalEndpoint) {
try {
const gcsUrl = new URL(url);
const externalUrl = new URL(this.externalEndpoint);
gcsUrl.hostname = externalUrl.hostname;
if (externalUrl.port) gcsUrl.port = externalUrl.port;
gcsUrl.protocol = externalUrl.protocol;
return gcsUrl.toString();
} catch (err) {
logger.warn(`Failed to replace URL with external endpoint: ${err}`);
}
}
// After: Extract into a helper function
private replaceWithExternalEndpoint(url: string): string {
if (!this.externalEndpoint) return url;
try {
const originalUrl = new URL(url);
const externalUrl = new URL(this.externalEndpoint);
originalUrl.hostname = externalUrl.hostname;
if (externalUrl.port) originalUrl.port = externalUrl.port;
originalUrl.protocol = externalUrl.protocol;
return originalUrl.toString();
} catch (err) {
logger.warn(`Failed to replace URL with external endpoint: ${err}`);
return url;
}
}
This approach improves code maintainability, readability, and ensures consistent behavior across the application.
Use structured logging with appropriate context and log levels to maximize the value of log messages. Include relevant metadata with each log message to facilitate filtering and debugging:
# Instead of:
logger.info("Processing item")
# Do this:
logger.info("Processing item", extra={"project_id": project.id, "item_type": item.type})
# Or with Sentry SDK:
sentry_sdk.set_context("operation_context", {"project_id": project.id, "item_type": item.type})
Instead of removing logging statements, consider lowering their level to DEBUG for infrequently needed information.
logger.exception() or exc_info=Truetry:
# operation
except ApiError as e:
# For non-critical errors that don't need immediate action
logger.warning("API operation failed", exc_info=True, extra={"operation": "fetch_data"})
# Or with Sentry:
sentry_sdk.set_context("operation_context", {"operation": "fetch_data"})
sentry_sdk.capture_exception(error=e, level="warning")
# Instead of:
lifecycle.record_halt(reason)
logger.info("Operation halted", extra={"reason": reason})
# Do this:
lifecycle.add_extras({"project_id": project.id})
lifecycle.record_halt(reason)
Following these practices improves troubleshooting efficiency, enables better log filtering, and ensures that logs provide maximum value for debugging production issues.
Always ensure database queries produce deterministic results by including explicit ORDER BY clauses when using LIMIT operations. This prevents unpredictable result ordering and potential data inconsistencies.
Key practices:
Example:
-- Incorrect: Non-deterministic ordering
SELECT *
FROM traces
WHERE project_id = ${projectId}
LIMIT 1;
-- Correct: Deterministic ordering
SELECT *
FROM traces
WHERE project_id = ${projectId}
ORDER BY event_ts DESC, id
LIMIT 1;
This practice is especially important when:
Replace all console.* calls with an appropriate structured logging framework using proper log levels. This ensures consistent logging patterns, better log management, and appropriate handling of different severity levels.
Key guidelines:
Example:
// Don't do this:
console.log("Processing batch:", JSON.stringify(batch));
console.warn("Redis client not available");
console.error("Failed to process:", error);
// Do this instead:
logger.info("Processing batch", { batch });
logger.warn("Redis client not available");
logger.error("Failed to process", { error });
This approach provides better log management, filtering capabilities, and consistent logging patterns across the codebase.
Design React hooks with performance and maintainability in mind by following these key principles:
Minimize useEffect usage - Update values directly when possible instead of using effects. Effects should only be used for true side effects, not for derived state updates.
Use the latest ref pattern for hook options to avoid unnecessary re-renders and ensure proper memoization:
function useCustomHook(options: Options) {
const optionsRef = useRef(options);
useLayoutEffect(() => {
optionsRef.current = options;
});
// Access latest options in callbacks
const handleChange = useCallback(() => {
const currentOptions = optionsRef.current;
// ... use currentOptions
}, []); // No options dependency needed
}
// Better approach
const handleSearchChange = useMemo(
() => debounce((value: string) => {
// Handle search
}),
[]
);
// Clean up on unmount
useEffect(() => {
return () => handleSearchChange.cancel();
}, []);
These patterns help avoid common pitfalls like unnecessary re-renders, stale closures, and complex dependency arrays while maintaining clean, performant React code.
When working with threads or thread pools in concurrent code, follow these practices to improve reliability and debuggability:
thread_name_prefix parameter. This makes debugging and monitoring much easier when issues arise.# Good
executor = ThreadPoolExecutor(max_workers=10, thread_name_prefix="data-processor")
# Good
thread = threading.Thread(name="result-sender", target=process_results)
# Avoid - unnamed threads make debugging difficult
executor = ThreadPoolExecutor(max_workers=10)
# Good - explicit timeout prevents hanging
thread.join(timeout=1.0)
future.result(timeout=5.0)
# Avoid - may block indefinitely
thread.join()
# Good for service threads that can terminate with the program
thread = threading.Thread(target=background_task, daemon=True)
as_completed, the timeout is already handled and doesn’t need to be repeated in the future.result() call.Following these practices will help avoid common threading pitfalls like application hang during shutdown, thread leaks, and difficult-to-debug concurrency issues.
Use HTTP status codes that accurately reflect the outcome of API operations. This improves API clarity and allows client applications to handle responses more intelligently.
Example for duplicate resources:
// For duplicate resource detection:
- expect(response.status).toBe(404);
+ expect(response.status).toBe(409);
Example for asynchronous processing:
PATCH: createAuthedAPIRoute({
name: "Update Single Trace",
// ...
fn: async ({ query, body, auth }) => {
// Process the request...
// Return 202 Accepted for async processing
return { status: 202, body: {
message: "Update accepted and will be processed"
}};
}
})
Design API parameters to be extensible and future-proof by using object arguments and explicit type definitions instead of boolean flags or simplistic checks. This approach makes APIs more maintainable and easier to evolve over time.
Key principles:
Example:
// ❌ Avoid rigid parameter design
function validate(condition: string): string {
// ...
}
// ✅ Use extensible object parameters
function validate({
condition,
context
}: {
condition: string,
context?: {
organization?: string,
// Easily extendable with new context properties
}
}): string {
// ...
}
// ❌ Avoid boolean flags that limit future options
interface ComponentProps {
fitMaxContent: boolean;
}
// ✅ Use string literals for extensible options
interface ComponentProps {
fit?: 'max-content' | 'min-content' | 'content'; // Extensible
}
Always preserve error context by using specific error types, safe error handling patterns, and meaningful error messages. This helps with debugging and maintains a clear error handling chain.
Key practices:
Example:
// ❌ Poor error handling
try {
const data = JSON.parse(response);
} catch (e) {
throw new Error("Failed to parse");
}
// ✅ Good error handling
try {
const data = JSON.parse(response);
} catch (e) {
throw new ValidationError(
`Failed to parse response: ${e instanceof Error ? e.message : String(e)}.
Response: ${response.substring(0, 100)}...`
);
}
// ✅ Good error handling with context
async function processItem(itemId: string) {
try {
const result = await backOff(
async () => await processWithRetries(itemId)
);
return result;
} catch (e) {
logger.error(
`Failed to process item ${itemId}: ${e instanceof Error ? e.message : String(e)}`
);
throw new ProcessingError(`Item processing failed: ${itemId}`, { cause: e });
}
}
Structure code to maximize readability and reduce cognitive load through proper formatting and control flow patterns. This includes breaking long function signatures across multiple lines, using early returns to reduce nesting levels, and extracting complex boolean expressions into well-named functions.
Key practices:
Example of improved readability through early returns:
// Instead of:
if t, ok := activeTargets[target.labels.Hash()]; ok {
// ... log here ...
}
// Prefer:
t, ok := activeTargets[target.labels.Hash()]
if !ok {
continue
}
// ... log here ...
Example of breaking long function signatures:
func NewZookeeperTreeCache(
conn *zk.Conn, path string,
events chan ZookeeperTreeCacheEvent,
logger *slog.Logger,
failureCounter prometheus.Counter,
numWatchers prometheus.Gauge,
) *ZookeeperTreeCache {
These practices make code easier to understand, debug, and maintain by reducing visual complexity and making the logical flow more apparent.
Avoid N+1 database queries by using appropriate Django ORM features like select_related(), prefetch_related(), and bulk operations. N+1 queries occur when you access related objects in a loop, causing additional database queries for each iteration.
Key practices:
Example - Instead of:
# Creates N+1 queries
for event_type in event_types:
event_type_snapshot = deepcopy(event_type)
nullify_id(event_type_snapshot)
event_type_snapshot.snuba_query = snuba_query_snapshot
event_type_snapshot.save()
Use:
# Single bulk operation
event_type_snapshots = []
for event_type in event_types:
event_type_snapshot = deepcopy(event_type)
nullify_id(event_type_snapshot)
event_type_snapshot.snuba_query = snuba_query_snapshot
event_type_snapshots.append(event_type_snapshot)
EventType.objects.bulk_create(event_type_snapshots)
# Or for related objects:
team_members = OrganizationMemberTeam.objects.filter(team=team).select_related('organizationmember')
Always protect sensitive data through proper encryption, secure cookies, and careful exception handling to prevent information leakage.
When handling sensitive data:
response.set_cookie(
settings.CSRF_COOKIE_NAME,
request.META.get("CSRF_COOKIE"),
secure=True,
httponly=True,
samesite='Lax',
domain=settings.CSRF_COOKIE_DOMAIN,
)
key = Fernet.generate_key() # Returns base64 encoded key ready for use
fernet = Fernet(key) encrypted_token = fernet.encrypt(token.encode(“utf-8”))
3. **Prevent information leakage in exceptions** - Avoid exposing stack traces or internal details to users:
```python
# Don't do this:
raise ParseError(detail=str(e)) # May leak sensitive information
# Instead, use controlled error messages:
raise ParseError(detail="Invalid or missing date range")
These practices help protect your application from various security vulnerabilities including session hijacking, credential exposure, and information disclosure.
Create metrics with descriptive, unique names that are distinguishable when viewed in monitoring tools. Avoid generic prefixes that make groups of related metrics indistinguishable from each other. Include specific actions or states in the metric name to clearly identify what’s being measured.
For example, instead of:
metrics.incr(
"group.update.http_response",
# This uses the same prefix as other methods, making metrics indistinguishable
)
Use:
metrics.incr(
"group.details.http_response", # More specific to this particular operation
tags={"method": "GET"}, # Add relevant dimensions for filtering
)
Additionally, place metrics as close as possible to the code they’re measuring to maintain clear correlations between code execution and performance data. Consider creating helper functions that provide consistent metric naming within a module or service.
When adding context to metrics via tags, be mindful of cardinality concerns - limit high-cardinality tags (like user IDs) to avoid performance issues with your monitoring system.
When implementing algorithms, prioritize simplicity and maintainability over premature optimization. Complex algorithms should be broken down into smaller, reusable components or replaced with simpler approaches when possible.
Key principles:
For example, instead of implementing complex bucket-level histogram manipulation:
// Complex approach - manipulating individual buckets
for bucketIndex, bucketVal := range resultHistogram.PositiveBuckets {
if bucketVal <= 0 {
continue
}
bucketStartVal := firstH.PositiveBuckets[bucketIndex]
bucketDelta := bucketVal * deltaScale
predictedAtStart := bucketStartVal - (bucketDelta / sampledInterval * durationToStart)
// ... more complex logic
}
Prefer a simpler count-based approach:
// Simpler approach - use overall count for decisions
durationToZero := sampledInterval * (samples.Histograms[0].H.Count / resultHistogram.Count)
if durationToZero < durationToStart {
durationToStart = durationToZero
}
This approach reduces complexity, improves maintainability, and often performs better while being easier to reason about and test.
Write tests that verify actual system behavior rather than implementation details by minimizing mock usage and focusing on real interactions. Excessive mocking can lead to brittle tests that pass even when core functionality is broken.
Key guidelines:
Example - Instead of:
@patch("sentry.api.endpoints.seer_rpc.integration_service.get_organization_integrations")
@patch("sentry.api.endpoints.seer_rpc.options.get")
def test_complex_scenario(self, mock_options, mock_integrations):
mock_integrations.return_value = []
mock_options.return_value = []
# Test implementation
Prefer:
@override_options({"feature.flag": True})
def test_complex_scenario(self):
# Create real integration
integration = self.create_integration(
organization=self.organization,
provider="github"
)
# Test actual behavior
This approach produces more reliable tests that better verify system correctness and are easier to maintain.
Always standardize configuration values to ensure consistency, reproducibility, and maintainability:
| curl -L https://github.com/golang-migrate/migrate/releases/download/v4.18.3/migrate.linux-amd64.tar.gz | tar xvz |
# Use environment variables with descriptive comments
CLICKHOUSE_MAX_MEMORY_USAGE: "${CLICKHOUSE_MEM:-800000000}" # Default ~800MB, override with CLICKHOUSE_MEM
This approach reduces maintenance burden, makes configuration changes safer and more deliberate, and provides better visibility into how systems are configured.
Prevent unnecessary recalculations and rerenders by properly memoizing expensive computations and derived values in React components. Pay special attention to dependency arrays and ensure they only include stable references.
Key points:
Example of proper memoization:
// Instead of this (causes unnecessary recalculations):
const sortBysString = JSON.stringify(sortBys);
// Do this:
const sortBysString = useMemo(() => JSON.stringify(sortBys), [sortBys]);
// For functions that depend on changing objects, extract stable values:
const handleSearchChange = useMemo(
() =>
debounce((newValue: string) => {
const currentParams = Object.fromEntries(searchParams.entries());
// ... rest of the logic
}, 500),
[setSearchParams] // Only depend on stable function reference
);
This optimization is particularly important for:
When writing tests, ensure your assertions thoroughly verify all relevant aspects of the functionality being tested. Go beyond the basic “happy path” checks and include assertions that verify:
Example 1: When testing a component that shouldn’t render certain elements:
// Instead of only checking that one text element isn't present:
expect(
screen.queryByText('Select from one of these suggestions')
).not.toBeInTheDocument();
// Also verify that no suggestion elements are rendered:
expect(screen.queryByText(/https:\/\/github\.com/)).not.toBeInTheDocument();
Example 2: When testing tabular data, verify column ordering:
// Don't just check that columns exist
expect(screen.getByText('http request_method')).toBeInTheDocument();
expect(screen.getByText('count span.duration')).toBeInTheDocument();
// Also verify the columns appear in the expected order
const headers = screen.getAllByRole('columnheader');
expect(headers[0].textContent).toBe('http request_method');
expect(headers[1].textContent).toBe('count span.duration');
Example 3: Use realistic test data rather than undefined values:
// Instead of letting values be undefined in tests
expect(screen.getByText('/settings/')).toHaveAttribute(
'href',
'/organizations/org-slug/traces/trace/undefined/?referrer=breadcrumbs'
);
// Provide valid mock data
const traceId = 'abcd1234efgh5678';
expect(screen.getByText('/settings/')).toHaveAttribute(
'href',
`/organizations/org-slug/traces/trace/${traceId}/?referrer=breadcrumbs`
);
Comprehensive assertions make tests more meaningful by ensuring they actually verify the intended behavior, catch regressions, and serve as documentation of expected functionality.
Write tests that focus on unique edge cases and avoid redundancy. When designing test suites:
Instead of redundant tests:
it('should return empty array for empty object input', () => {
expect(uniq({})).toStrictEqual([]);
});
it('should return empty array for object with properties', () => {
expect(uniq({key: 'value'})).toStrictEqual([]);
});
Write a more meaningful test:
it('should return empty array for array-like objects that are not arrays', () => {
const arrayLike = {0: 'a', 1: 'b', length: 2};
expect(uniq(arrayLike as any)).toStrictEqual([]);
});
For test fixtures, place them in dedicated directories (e.g., tests/js/fixtures/) and design them to accept parameters for customization:
// tests/js/fixtures/tabularColumn.ts
export const TabularColumnFixture = (params: Partial<TabularColumn> = {}): TabularColumn => ({
key: 'default-key',
name: 'Default Name',
type: 'string',
...params,
});
This approach improves test maintainability and ensures your test suite provides meaningful coverage without unnecessary bloat.
When working with concurrent code, carefully manage shared resources and synchronization to prevent race conditions, deadlocks, and resource leaks:
// BAD: Accessing map from multiple goroutines without synchronization
go func() {
runErrs[testServer.id] = err // Race condition!
}()
// GOOD: Use mutex to protect shared map
var mu sync.Mutex
go func() {
mu.Lock()
defer mu.Unlock()
runErrs[testServer.id] = err
}()
// BAD: Modifying the same object across goroutines
go func() {
// Both goroutines modify the same object
created.Status = "processing"
}()
// GOOD: Create a copy for the goroutine to modify
go func() {
createdCopy := created.DeepCopyObject()
createdCopy.Status = "processing"
}()
defer to ensure resources are always cleaned up, even in error paths.// BAD: Channel might not be closed on some error paths
if err != nil {
return // events channel never closed!
}
close(events)
// GOOD: Guarantee cleanup with defer
defer close(events)
// BAD: Using Background context disconnects from parent cancellation
go func() {
ctx := context.Background() // Ignores parent cancellation signals
// long-running operation that won't be cancelled
}()
// GOOD: Propagate the original context
go func() {
// This operation will be cancelled when parent context is cancelled
select {
case <-ctx.Done():
return
case <-time.After(5 * time.Second):
// continue processing
}
}()
// BAD: Multiple goroutines racing to send to channel can cause ordering issues
go func() {
stream <- event // May race with other senders
}()
// GOOD: Use a central goroutine to manage channel sends
eventQueue := make(chan Event, 100)
go func() {
for event := range eventQueue {
stream <- event // Maintains ordering
}
}()
// BAD: Sending value for signaling
st.stateRunnerShutdown <- struct{}{}
// GOOD: Closing channel for signaling
close(st.stateRunnerShutdown)
Design functions for maximum reusability, clarity, and maintainability. Follow these principles:
// Avoid
function shouldTextOverflow(
fieldType: FieldType,
cellType: TableCellDisplayMode,
textWrap: boolean,
cellInspect: boolean
): boolean {
return fieldType === FieldType.string && cellType !== TableCellDisplayMode.Image && !textWrap && !cellInspect;
}
// Prefer
function shouldTextOverflow(field: Field): boolean {
const cellType = getCellOptions(field).type;
const textWrap = shouldTextWrap(field);
const cellInspect = isCellInspectEnabled(field);
return field.type === FieldType.string && cellType !== TableCellDisplayMode.Image && !textWrap && !cellInspect;
}
Extract reusable functions instead of duplicating code: When similar code appears in multiple places, extract it into a shared function. Before writing new utility functions, check if similar functionality already exists in the codebase.
Use named functions over anonymous functions: Replace self-executing anonymous functions with named functions for better readability and debuggability.
// Avoid
importPromises[pluginId] = (async () => {
// implementation
})();
// Prefer
async function loadPlugin(pluginId: string): Promise<AppPlugin> {
// implementation
}
importPromises[pluginId] = loadPlugin(pluginId);
Define and document a clear precedence order when configurations come from multiple sources (code parameters, environment variables, defaults). Typically, explicit parameters should take precedence over environment variables, which should take precedence over defaults.
For collection-based configurations, decide whether sources should be merged (additive) or replaced:
# Additive approach (params + env vars)
span_exporters = _import_exporters(
span_exporter_names + _get_exporter_names("traces"),
# ...
)
# Replacement approach (params override env vars)
headers = self._headers or tuple()
# Instead of: headers = tuple(_OTLP_GRPC_HEADERS)
# Feature flags for breaking changes
if not environ.get("OTEL_PYTHON_EXPERIMENTAL_DISABLE_PROMETHEUS_UNIT_NORMALIZATION"):
# Apply new normalization behavior
metric_name = sanitize_full_name(metric.name)
metric_unit = map_unit(metric.unit)
else:
# Use legacy behavior
metric_name = self._sanitize(metric.name)
Document the precedence behavior clearly, especially for components that might be configured through multiple mechanisms. Consider providing temporary opt-out mechanisms via environment variables when making breaking configuration changes.
Extract logic that doesn’t render JSX into custom hooks instead of components. Components that don’t return markup create unnecessary abstraction layers and can lead to harder-to-follow data flows.
When a module only manages state, side effects, or logic without rendering UI elements, implement it as a custom hook:
// Instead of this:
export function SessionExpiryMonitor({ warningMinutes = 5 }) {
const [hasShownWarning, setHasShownWarning] = useState(false);
useEffect(() => {
// monitoring logic
}, [warningMinutes, hasShownWarning]);
return null; // No JSX rendered
}
// Prefer this:
export function useSessionExpiryMonitor(warningMinutes = 5) {
const [hasShownWarning, setHasShownWarning] = useState(false);
useEffect(() => {
// monitoring logic
}, [warningMinutes, hasShownWarning]);
}
For data-dependent components, split them into container and presentational parts to avoid rendering with stale data. This pattern improves predictability by separating data fetching from rendering.
Additionally, ensure proper state updates when modifying child components. If you update child state in a way that should affect the parent, make sure to update the parent state explicitly:
// When modifying a child component's state
sceneGridLayout.setState({
children: [newGridItem, ...sceneGridLayout.state.children],
});
// Don't forget to update the parent state if needed
this.setState({ body: sceneGridLayout });
When handling router state, prefer React-specific hooks like useLocation over direct browser API subscriptions, as they better integrate with React’s lifecycle and re-rendering mechanisms.
Use appropriate mocking approaches for different dependencies in tests to ensure accurate test results and prevent false positives. Different types of dependencies require different mocking strategies:
window.open):
// Define and mock the property before using it
// @ts-expect-error global.open should return a Window, but is not implemented in js-dom.
const openSpy = jest.spyOn(global, 'open').mockReturnValue(true);
afterEach(() => { jest.resetAllMocks(); });
3. For module dependencies, use dedicated mocks rather than actual imports:
```javascript
// Instead of using setupDataSources which imports actual modules:
const dsServer = new MockDataSourceSrv(datasources) as unknown as DataSourceSrv;
Using precise mocking strategies improves test reliability, prevents unexpected behavior from external dependencies, and ensures tests validate exactly what you intend to test.
All user-facing text in UI components should be properly documented for translation using the application’s internationalization (i18n) framework. This ensures that the application can be easily localized for different regions and improves maintainability.
When adding text to a component:
import { t, Trans } from '@grafana/i18n';t function with specific, namespaced translation keysExample:
// DON'T do this (hardcoded strings):
const openPrText = isFolder
? 'A new folder has been created in a pull request in GitHub.'
: 'This dashboard is loaded from a pull request in GitHub.';
// DO this instead (properly documented for translation):
const openPrText = isFolder
? t('provisioned-dashboard-preview-banner.title-folder-created', 'A new folder has been created in a pull request in GitHub.')
: t('provisioned-dashboard-preview-banner.title-dashboard-loaded', 'This dashboard is loaded from a pull request in GitHub.');
// For button text and other UI elements:
<Button
icon="angle-left"
aria-label={t('return-to-previous.aria-label', 'Return to previous page')}
>
{t('return-to-previous.button-text', 'Back to {destination}', { destination: shortenTitle(children) })}
</Button>
Using standardized translation keys with descriptive namespaces improves code documentation and makes it easier for translators to understand the context of each string.
When a function has 3 or more parameters, use object destructuring instead of positional arguments. This improves code readability by making parameter names explicit at call sites and making the function signature more maintainable as parameters are added or modified.
Instead of this:
export const wrapWithPluginContext = <T,>(
pluginId: string,
extensionTitle: string,
Component: React.ComponentType<T>,
log: ExtensionsLog
) => {
// Implementation
}
// Call site
wrapWithPluginContext('my-plugin', 'My Extension', MyComponent, logger);
Do this:
export const wrapWithPluginContext = <T,>({
pluginId,
extensionTitle,
Component,
log,
}: {
pluginId: string;
extensionTitle: string;
Component: React.ComponentType<T>;
log: ExtensionsLog;
}) => {
// Implementation
}
// Call site
wrapWithPluginContext({
pluginId: 'my-plugin',
extensionTitle: 'My Extension',
Component: MyComponent,
log: logger,
});
This approach makes it immediately clear what each parameter represents, allows for future parameter additions without breaking existing call sites, and makes the code easier to read and maintain.
Use naming conventions that are both consistent with existing patterns and descriptively clear about purpose. Follow established patterns in the codebase for similar entities while ensuring names are specific and self-explanatory.
Example from Prisma schema:
// Avoid this:
model Project {
// Inconsistent casing (camelCase vs PascalCase)
Dashboard Dashboard[]
actions Action[] // Inconsistent
// Generic, unclear name
SavedView SavedView[] // Too generic
}
// Prefer this:
model Project {
// Consistent casing pattern
Dashboard Dashboard[]
Actions Action[] // Consistent
// Specific, descriptive name
TableViewPreset TableViewPreset[] // Clear purpose
}
When adding new fields, models, or variables, check existing related elements and follow their naming pattern. If existing names are unclear, consider refactoring them to be more descriptive about their function and context.
Always validate objects for null/nil before accessing their properties, and establish consistent patterns for handling null values throughout the codebase. This prevents nil pointer dereferences and ensures predictable behavior.
Three key practices:
// Good: Check for nil first if returnedObj == nil { return nil, errors.New(“cannot enrich nil object”) } accessorReturned, err := meta.Accessor(returnedObj)
2. **Use proper objects instead of nil values:**
```go
// Bad: Using nil objects that may cause dereference errors
return newRecordingRule(context.Background(), models.AlertRuleKeyWithGroup{}, 0, nil, nil, st, log.NewNopLogger(), nil, nil, writer.FakeWriter{}, nil, nil)
// Good: Use proper initialized objects
mockClock := clock.NewMock()
retryConfig := RetryConfig{
MaxRetries: 3,
RetryInterval: time.Second,
}
return newRecordingRule(context.Background(), models.AlertRuleKeyWithGroup{}, retryConfig, mockClock, nil, st, log.NewNopLogger(), nil, nil, writer.FakeWriter{}, nil, nil)
When handling special cases like empty strings or missing values, document the expected behavior and ensure all developers follow the same patterns.
Always use proper quoting in shell scripts (especially GitHub Actions workflows) to prevent word splitting, globbing, and ensure correct variable expansion:
# Incorrect
go list -f '{{.Dir}}/...' -m | xargs go test -short -covermode=atomic -timeout=5m
# Correct
go list -f '{{.Dir}}/...' -m | xargs bash -c "go test -short -covermode=atomic -timeout=5m"
# Incorrect - variables won't expand in single quotes
echo 'Value: $SOME_VAR'
# Correct - allows variable expansion
echo "Value: $SOME_VAR"
# Incorrect
if [ $status == success ]; then
# Correct
if [ "$status" == "success" ]; then
These practices prevent common shell scripting errors that tools like shellcheck will flag (SC2046, SC2016, SC2086), resulting in more robust and predictable scripts.
Always use null-safe access patterns when dealing with potentially undefined values to prevent runtime errors and improve code robustness. This includes:
?.) for nested property access:
// Instead of assuming objects/properties exist:
managedBy: ManagerKind[item.metadata.annotations['grafana.com/managed-by']]
// Use optional chaining:
managedBy: item.metadata?.annotations?.[AnnoKeyManagerKind]
// Instead of assuming rule exists:
const isProvisioned = rulerRuleType.grafana.rule(rule) && Boolean(rule.grafana_alert.provenance);
// Use conditional checking:
const isProvisioned = rule ? isProvisionedRule(rule) : false;
// Instead of special values or undefined:
return metrics.length === 0 ? MATCH_ALL_LABELS_STR : ...;
// Return empty collections:
return metrics.length === 0 ? [] : [...];
// Instead of assuming the type is always present:
const parsed = loadAll(raw) as Array<{ kind: string }>;
// Account for potential unknown fields and nullability:
const parsed = loadAll(raw) as Array<Record<string, unknown> & { kind: string }>;
This approach makes code more defensive and easier to maintain by explicitly handling null cases and preventing “cannot read property of undefined” runtime errors.
When implementing or testing algorithms, ensure mathematical relationships and constraints are maintained throughout the computation. Test data should have plausible values that reflect real-world mathematical relationships, and algorithmic edge cases should be carefully handled to prevent mathematical inconsistencies.
Key areas to focus on:
Test data consistency: Ensure test values have plausible mathematical relationships. For example, if testing histogram data, the sum field should reflect realistic values based on bucket distributions rather than arbitrary numbers.
Interpolation and extrapolation boundaries: When algorithms involve interpolation or extrapolation (like counter increases), carefully choose test intervals and handle edge cases where extrapolation might produce invalid results (e.g., negative values for counters).
Numerical precision awareness: Be mindful of floating-point precision issues, especially in aggregation algorithms. Direct calculations may be more precise than incremental approaches for certain operations.
Example from histogram testing:
# Bad: Arbitrary sum value that doesn't match bucket distribution
h_test {{schema:0 count:34 sum:5 buckets:[2 4 8 16]}}
# Good: Sum value that reflects realistic bucket distribution
h_test {{schema:0 count:34 sum:170 buckets:[2 4 8 16]}}
This approach helps ensure algorithms behave correctly under various mathematical conditions and produce results that maintain expected mathematical properties.
Implement comprehensive validation for environment variables using a schema validation library (e.g., Zod). Include proper typing, conditional validation rules, and sensible defaults. This ensures configuration errors are caught early and prevents runtime issues.
Key practices:
Example:
const EnvSchema = z.object({
// Use appropriate types instead of string enums
DEBUG_MODE: z.coerce.boolean().default(false),
// Add conditional validation for dependent configs
S3_MEDIA_UPLOAD_SSE: z.enum(["AES256", "aws:kms"]).optional(),
S3_MEDIA_UPLOAD_KMS_KEY_ID: z.string().optional()
.refine((val, ctx) => {
if (ctx.parent.S3_MEDIA_UPLOAD_SSE === "aws:kms" && !val) {
return false;
}
return true;
}, "KMS key ID required when using aws:kms encryption"),
// Provide fallbacks for optional configs
SENTRY_RELEASE: z.string().optional()
.default(process.env.BUILD_ID ?? "unknown")
});
Use clear, consistent naming patterns for variables and event handlers. Avoid abbreviations unless they are widely understood standards. For event handlers, follow the pattern ‘on’ + ‘action’ + ‘object’.
Good examples:
// Event handler naming
onSortChange: (sort: Sort) => void; // ✓ Clear and follows pattern
onColumnSortChange: () => void; // ✗ Redundant, object implied
// Variable naming
const currentSort = useState<Sort>(); // ✓ Clear and explicit
const curSort = useState<Sort>(); // ✗ Unnecessary abbreviation
// Event handler pattern
onChangePassword() // ✓ on + action + object
onPasswordChange() // ✗ Inconsistent pattern
This promotes code readability and maintains consistency across the codebase. When multiple developers follow the same naming patterns, it becomes easier to understand and maintain the code.
When designing component APIs, strive to find the optimal balance between providing flexible configuration options and maintaining simplicity. Start with essential functionality before adding complex features that may complicate the API surface.
In Discussion 0, we see how a PillCell component initially had a simpler color implementation:
// Simple approach first
function getPillColor(pill: string, cellOptions: TableCellRendererProps['cellOptions']): string {
if (!isPillCellOptions(cellOptions)) {
return getDeterministicColor(pill);
}
const colorMode = cellOptions.colorMode || 'auto';
// Fixed color mode (highest priority)
if (colorMode === 'fixed' && cellOptions.color) {
// Simple implementation
}
}
The team later added more sophisticated key-value mapping for colors when there was clear need, rather than over-complicating the initial implementation.
Similarly, in Discussion 3, a field selection API became overly complex when trying to handle all edge cases: “When the names collide choosing the correct field from the correct dataframe could be problematic.”
Consider these principles when designing APIs:
This approach prevents premature complexity while ensuring your API can evolve to meet genuine requirements.
Ensure sorting functions are named to accurately reflect their behavior and implement consistent sorting approaches throughout the codebase. When implementing sort operations:
sortByResourceVersion instead of generic sortKeys)Inconsistent or misleadingly named sorting functions can lead to unexpected results and bugs when code evolves. Consider this example:
// Unclear - suggests sorting by all key fields
func sortHistoryKeys(filteredKeys []DataKey, sortAscending bool) {
// But actually only sorts by ResourceVersion
sort.Slice(filteredKeys, func(i, j int) bool {
if sortAscending {
return filteredKeys[i].ResourceVersion < filteredKeys[j].ResourceVersion
}
return filteredKeys[i].ResourceVersion > filteredKeys[j].ResourceVersion
})
}
// Better - clearly indicates sorting by ResourceVersion
func sortHistoryKeysByResourceVersion(filteredKeys []DataKey, sortAscending bool) {
sort.Slice(filteredKeys, func(i, j int) bool {
if sortAscending {
return filteredKeys[i].ResourceVersion < filteredKeys[j].ResourceVersion
}
return filteredKeys[i].ResourceVersion > filteredKeys[j].ResourceVersion
})
}
When multiple components need to sort the same data structures, ensure they use the same sorting logic to prevent inconsistent results across the system.
Write tests that focus on behavior rather than implementation details to create more stable and maintainable test suites. Avoid using arbitrary timeouts and assertions that frequently break with implementation changes.
Instead of using arbitrary timeouts:
// Avoid this:
await searchInput.click();
await page.keyboard.type('Datasource tests - MySQL');
await page.waitForTimeout(300); // Brittle: might be too short or too long
// Prefer this:
await searchInput.click();
await page.keyboard.type('Datasource tests - MySQL');
await expect(rowGroup).toHaveCount(1); // Will auto-retry until condition is met
await expect(rows).toHaveCount(expectedLength);
Instead of asserting implementation details:
// Avoid this:
expect(scene.state.$behaviors).toHaveLength(6); // Brittle: breaks when behavior count changes
// Prefer this:
expect(scene.state.$behaviors.find(b => b instanceof CursorSyncBehavior)).toBeDefined(); // Tests actual functionality
By focusing tests on behavior and using the testing framework’s built-in retry and assertion mechanisms, you’ll create more reliable tests that require less maintenance as your codebase evolves.
Carefully manage configuration options, including feature flags, to balance flexibility with maintainability:
// Good: Feature flag for gradual rollout
if featuremgmt.AnyEnabled(&featuremgmt.FeatureManager{}, featuremgmt.FlagNewFeature) {
providers = []app.Provider{playlistAppProvider, shortURLAppProvider}
} else {
providers = []app.Provider{playlistAppProvider}
}
// Better when possible: Backward compatible default behavior
options := getCookieOptions()
if featuremgmt.AnyEnabled(&featuremgmt.FeatureManager{}, featuremgmt.FlagPanelExporterCookieDomain) {
// Only modify behavior when flag is enabled
} else {
options.Domain = "" // Maintains previous behavior
}
HideFromDocs: true for internal features)Always ensure configuration documentation accurately reflects the current system capabilities, limitations, and feature availability status. When documenting configuration options:
Example:
# Incorrect (outdated) documentation
;footer_hide = true # Use this to hide the footer
# Correct (updated) documentation
# footer_hide is no longer available
;footer_text = "Custom footer text" # Set custom text for the footer
;footer_logo = "logo.png" # Set custom logo for the footer
# If neither is set, the default Grafana footer is displayed
Accurate configuration documentation prevents user confusion, reduces support requests, and ensures smooth system operation across different environments.
Choose efficient implementation patterns that improve performance. Apply these optimizations throughout your code:
// More efficient: store the reference timeField := timeFields[frame]
2. **Pre-allocate when size is known**: For collections with predictable sizes, pre-allocate to avoid resizing.
```go
// Pre-allocate with known capacity
rows := make([]row, 0, totalRows)
b.ResetTimer() before the actual code being benchmarked to exclude setup time.
func BenchmarkOperation(b *testing.B) {
// Setup code
data := prepareTestData()
// Start measuring only the operation we care about
b.ResetTimer()
for i := 0; i < b.N; i++ {
performOperation(data)
}
}
// More efficient - using regular inserts after truncation if _, err := sess.Exec(“DELETE FROM alert_instance”); err != nil { return err } // Then using regular inserts
Regularly profile your code with realistic data volumes to identify and address bottlenecks.
---
## Feature toggle lifecycle
<!-- source: grafana/grafana | topic: Configurations | language: TypeScript | updated: 2025-07-02 -->
When working with feature toggles and configuration flags, manage their entire lifecycle carefully to prevent breaking changes and compatibility issues.
Guidelines:
1. Document feature toggles when introduced, including purpose, scope, and expected removal timeline
2. Consider backward compatibility impacts before removing feature toggles
3. Ensure that configuration changes maintain compatibility with supported versions of dependent services
4. Provide migration paths when deprecating or removing configuration options
Example of proper feature toggle handling:
```typescript
// When introducing a feature toggle:
if (config.featureToggles.myNewFeature) {
// New behavior with documentation
// NOTE: This toggle supports X functionality and will be removed in version Y
// when the feature becomes default behavior
}
// When maintaining backward compatibility:
if (config.featureToggles.legacyToggle || !options?.streamSelector) {
return this.newBehavior(options);
} else {
// Maintain compatibility with older versions
const data = await this.legacyBehavior(options);
return adaptToNewFormat(data);
}
Breaking changes to configuration defaults or removing feature toggles should be properly documented and typically done during major version updates, not in patch or minor releases.
Tests should verify actual behavior rather than just successful execution. When writing assertions, favor precise equality checks that verify the exact content and ordering rather than loose containment checks.
For tests involving external systems or complex interactions, verify what was actually sent or received rather than just that an operation completed successfully. This ensures the test catches subtle bugs in the data being transmitted.
Example:
// Weak assertion - only checks that elements exist but not complete set or order
require.Contains(t, names, "resource-1")
require.Contains(t, names, "resource-2")
// Strong assertion - verifies exact content and ordering
require.Equal(t, []string{"resource-1", "resource-2"}, names)
Similarly, when testing server interactions, verify the exact payload sent to the server:
// Add verification of what the server received
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
// Capture and verify what was sent to the server
require.NoError(t, json.NewDecoder(r.Body).Decode(&configSent))
require.Equal(t, expectedConfig, configSent)
// Rest of the handler
}))
Precise assertions make tests more reliable at catching regressions and unintended changes in behavior.
Implement preliminary validation checks before executing expensive operations to avoid unnecessary processing. When handling operations that may be computationally intensive (like text layout calculations, data transformations, or complex rendering), add targeted checks that can early-exit or skip work when the operation would have no effect or be redundant.
For example, when calculating text wrapping dimensions:
// Before optimization
const calcRowHeight = (text: string, cellWidth: number, defaultHeight: number) => {
const numLines = count(text, cellWidth);
const totalHeight = numLines * TABLE.LINE_HEIGHT + 2 * TABLE.CELL_PADDING;
return Math.max(totalHeight, defaultHeight);
};
// After optimization
const calcRowHeight = (text: string, cellWidth: number, defaultHeight: number) => {
// Skip expensive line counting if text has no spaces (can't wrap)
if (!text || !/[\s]+/.test(text)) {
return defaultHeight;
}
const numLines = count(text, cellWidth);
const totalHeight = numLines * TABLE.LINE_HEIGHT + 2 * TABLE.CELL_PADDING;
return Math.max(totalHeight, defaultHeight);
};
Always validate optimization checks with performance profiling on representative data sets to ensure they actually improve performance and don’t introduce regressions.
Identify and extract repeated code patterns into well-named functions to improve readability, maintainability, and testability. Look for blocks of code that appear in multiple places or complex operations that can be encapsulated with a descriptive function name.
Examples:
// Create a helper function: func writeObject(t *testing.T, backend *Backend, testObj Object) { metaAccessor, err := utils.MetaAccessor(testObj) require.NoError(t, err) writeEvent := WriteEvent{…} // etc. }
2. For common API patterns with similar behavior:
```go
// Instead of repeating client initialization logic:
func NewResourceClient(conn grpc.ClientConnInterface) ResourceClient {
return &resourceClient{...}
}
func NewAuthlessResourceClient(cc grpc.ClientConnInterface) ResourceClient {
return newResourceClient(cc)
}
func NewLegacyResourceClient(cc grpc.ClientConnInterface) ResourceClient {
return newResourceClient(cc)
}
// Instead of repeating timing and metrics in each CRUD method:
func (s *store) withMetrics(ctx context.Context, operation string, fn func() error) error {
start := time.Now()
err := fn()
s.metrics.ObserveOperation(operation, time.Since(start), err == nil)
return err
}
This approach makes the main code paths clearer, easier to maintain, and reduces the risk of inconsistencies. When extracting functions, use descriptive names that indicate what the function does rather than how it works.
Ensure documentation adheres to established style guidelines and technical standards:
// Incorrect
Grafana's dashboard features allow users to visualize data.
Then we can use the following query for our graph panel.
// Correct
The dashboard features in Grafana allow users to visualize data.
To call this stored procedure from a graph panel, use the following query.
[Supported Resources](#supported-resources)
{{< relref "./path/to/page" >}}
Identify and cache results of computationally expensive or repetitive operations instead of recalculating them multiple times. This includes function calls, date object creation, data transformations, and resolved promises.
Examples of operations to cache:
// Before: Inefficient - recalculating for each prompt
await Promise.all(
resolvedPrompts.map(async (prompt) =>
promptChangeEventSourcing(
await promptService.resolvePrompt(prompt), // Redundant async call
// After: Efficient - reusing already resolved data
await Promise.all(
resolvedPrompts.map(async (prompt) =>
promptChangeEventSourcing(
prompt, // Using already resolved prompt
// Before: Inefficient - repeated function call
const maxTemperature = getDefaultAdapterParams(selectedProviderApiKey.adapter).maxTemperature;
const temperature = getDefaultAdapterParams(selectedProviderApiKey.adapter).temperature;
const maxTokens = getDefaultAdapterParams(selectedProviderApiKey.adapter).max_tokens;
// After: Efficient - computing once and reusing
const adapterParams = getDefaultAdapterParams(selectedProviderApiKey.adapter);
const maxTemperature = adapterParams.maxTemperature;
const temperature = adapterParams.temperature;
const maxTokens = adapterParams.max_tokens;
Look for patterns where the same operation is performed repeatedly with the same inputs, especially in loops, map functions, or closely related code blocks.
Avoid subjective terms, unexplained jargon, and assumptions about user knowledge in documentation. Replace subjective language like “obvious” with descriptive explanations, define technical terms that may not be familiar to all readers, and break down compact technical descriptions into more accessible language.
Examples of improvements:
This approach makes documentation more inclusive and accessible to users who may be learning or feeling overwhelmed, rather than assuming expert-level familiarity with concepts.
Always close resources (files, streams, connections) and properly handle their Close() errors. For read operations, Close() errors can be ignored if the primary operation (like JSON parsing) succeeded. For write operations, always check and propagate Close() errors.
Example:
// Bad:
data, err := io.ReadAll(reader)
reader.Close() // Error ignored!
// Good for reads:
defer reader.Close()
var result MyStruct
err := json.NewDecoder(reader).Decode(&result)
return result, err
// Good for writes:
writer := getWriter()
defer func() {
if closeErr := writer.Close(); err == nil {
err = closeErr // Only propagate Close error if no other error occurred
}
}()
return writer.Write(data)
This practice prevents resource leaks and ensures data integrity, especially for write operations where Close() errors could indicate incomplete writes or corruption.
Prefer direct filtering or specialized service methods over complex JOIN operations, especially when maintaining or refactoring database access patterns. JOINs across multiple tables can impact performance and make code more difficult to maintain, particularly during architectural migrations.
When possible:
For example, instead of:
-- Complex join approach
SELECT le.* FROM library_element le
LEFT JOIN folder f ON le.folder_uid = f.uid AND le.org_id = f.org_id
WHERE f.title LIKE '%search_term%'
Consider:
// Using specialized service methods
folderUIDs, err := folderService.SearchFolders(c, searchQuery)
// Then directly filter using the results
builder.Write("WHERE le.folder_uid IN (?)", folderUIDs)
When handling cross-database compatibility, remember that complex operations like multi-column WHERE NOT IN statements are database-specific extensions. Structure your queries to maintain compatibility or implement database-specific code paths with proper fallbacks.
Choose names that lead with their primary purpose or category, followed by specific details. This makes code more discoverable and self-documenting by grouping related items and clearly conveying intent.
Key guidelines:
Example:
// Less clear:
func sanitizeGrpcHeaderValue(value string) string {}
func dashboardImageSharing() {}
// More clear:
func sanitizeHTTPHeaderValueForGRPC(value string) string {}
func sharingDashboardImage() {}
This pattern helps when:
Authentication notifications must include specific context about the triggering action, clear instructions for legitimate use, expiration details, and explicit steps to take if the action was unauthorized. This enhances security by helping users identify potential account compromises and take appropriate action.
Example:
<p>You've initiated an account merger which requires verification. Please use the code below to confirm:</p>
<p>{{code}}</p>
<p>This code expires in 30 minutes.</p>
<p>If you didn't attempt this action, please secure your account immediately and contact support@sentry.io.</p>
Rather than the less secure alternative:
<p>Here is the verification code you requested. It expires in 30 minutes.</p>
<p>{{code}}</p>
<p>If you weren't expecting this email, please ignore it.</p>
When implementing observability features (logs, metrics, traces), use consistent semantic conventions to ensure data can be correlated and analyzed efficiently across the entire system.
Define core observability attributes in foundational packages that can be imported by other components. This creates a single source of truth for naming conventions.
Use semantic conventions for all observability signals including logs, metrics, and traces:
// Instead of manually adding attributes with varying names:
reqContext.Logger = reqContext.Logger.New("userId", reqContext.UserID, "orgId", reqContext.OrgID, "uname", reqContext.Login)
// Use standardized attribute helpers:
reqContext.Logger = reqContext.Logger.New(log.Attributes(
o11ysemconv.UserID.LogKV(reqContext.UserID),
o11ysemconv.OrgID.LogKV(reqContext.OrgID),
o11ysemconv.Username.LogKV(reqContext.Login),
))
Create consistent naming conventions for metrics that reflect their purpose rather than implementation details. For example, use remote_alertmanager_syncs_total instead of remote_secondary_forked_alertmanager_syncs_total.
Use centralized metrics registration to prevent duplicate registrations:
// Instead of:
prometheus.NewRegistry()
// or
prometheus.DefaultRegisterer
// Use the provided utility:
metrics.ProvideRegisterer()
Following these conventions ensures that observability data remains consistent and queryable across the entire system, making it easier to correlate information during debugging and analysis.
Always explicitly declare optional types in interfaces and function parameters rather than relying on runtime checks or using any. This improves type safety, makes null/undefined handling requirements clear, and prevents runtime errors.
Example:
// ❌ Poor - Implicit optionality, uses any
interface Props {
data: any;
config: any;
}
// ✅ Good - Explicit optional types
interface Props {
data: DataType;
config?: ConfigType; // Explicitly optional
extraction?: Extraction; // Clearly shows optionality
}
Benefits:
When implementing components or functions with optional parameters, ensure the types accurately reflect which values could be undefined. This creates a contract that makes null handling requirements obvious to consumers.
Ensure all plugin configuration files include the complete set of required fields specific to their plugin type. Missing configuration fields can cause unexpected behavior or failures when the plugin is used.
For example, all decoupled data sources must include the “executable” field in their plugin.json:
{
"id": "your-datasource",
"name": "Your Datasource",
"type": "datasource",
"executable": "gpx_your_datasource",
// other configuration...
}
When structuring configuration keys in localization files, avoid duplicating namespace information that is already represented in the file path. Use clear and concise language in configuration documentation, especially for deprecation notices or important instructions.
Reference existing plugins of the same type to ensure consistency in configuration structure. This helps maintain uniformity across the codebase and ensures all necessary configurations are properly declared.
When implementing or modifying APIs, ensure backward compatibility across versions and client interfaces. Carefully evaluate field structures, method visibility, and response formats for consistency.
When fields differ between API versions:
// Instead of directly checking version-specific fields
if (fromCache && fromCache.state.version === rsp?.dashboard.version) {
// ...
}
// Use a more robust check that works across API versions
if (fromCache && fromCache.state.meta.updated === rsp?.meta.updated) {
// ...
}
Before making methods private or changing signatures, verify there are no external consumers. Generate API clients in a way that accommodates both open source and enterprise code organization. When supporting multiple API versions (e.g., v1, v2, beta), implement fallback mechanisms and thoroughly test each version with appropriate feature toggles.
Document any breaking changes clearly, including migration paths for consumers. Consider providing adapter patterns or polyfills during transition periods to ease adoption of new API versions.
Design API endpoints with consistent, unambiguous URL patterns to improve usability and maintainability. Follow these principles:
# Bad: Generic parameter names
r"^toolbar/(?P<project_id_or_slug>[^/]+)/(?P<project_id_or_slug>[^/]+)/"
# Good: Specific parameter names that reflect their purpose
r"^toolbar/(?P<organization_slug>[^/]+)/(?P<project_id_or_slug>[^/]+)/"
-summary suffix instead of nested paths:
# Potentially ambiguous: Is "summary" an ID or an action?
r"^(?P<organization_id_or_slug>[^\/]+)/user-feedback/summary/$"
# Better: Clearly distinguishes the summary action
r"^(?P<organization_id_or_slug>[^\/]+)/feedback-summary/$"
# Support both formats:
# - project=1&project=2 (multiple occurrences)
# - project=[1,2] (array format)
# In URL documentation:
":qparam list[int] project: list of project IDs to filter by"
# In implementation:
project_ids = request.GET.getlist('project')
Consistent URL design makes your API more intuitive for clients to use and easier to maintain as it evolves.
Log messages should provide sufficient context to be meaningful and actionable while following structured logging patterns. Include relevant metrics, ratios, or identifiers that help diagnose issues. Use semantic conventions and structured key-value pairs rather than embedding values in message strings.
Example (Before):
logger.Info("empty refid found", "query_count", len(queries))
Example (After):
logger.Info("empty refid found", "empty_count", 1, "query_count", len(queries))
For error logging, use structured logging with semantic conventions:
// Prefer this
log.ErrorReason(gfErr.Reason)
// Over this
log.Info("errorReason", gfErr.Reason, "error", gfErr.LogMessage)
Well-structured logs with proper context improve observability and make debugging more efficient across the entire system.
When writing tests that require network services (HTTP, gRPC, or custom protocols), use port 0 to let the operating system assign available ports rather than hard-coding specific port numbers. Hard-coded ports can cause conflicts when multiple tests run simultaneously, leading to flaky tests.
After binding to the dynamic port, query the listener to determine which port was assigned for subsequent connection attempts:
// Instead of this:
cfg.HTTPPort = "13000"
cfg.GRPCServer.Address = "127.0.0.1:20000"
// Do this:
listener, err := net.Listen("tcp", "127.0.0.1:0")
require.NoError(t, err)
port := listener.Addr().(*net.TCPAddr).Port
cfg.HTTPPort = strconv.Itoa(port)
// And for gRPC:
grpcListener, err := net.Listen("tcp", "127.0.0.1:0")
require.NoError(t, err)
grpcPort := grpcListener.Addr().(*net.TCPAddr).Port
cfg.GRPCServer.Address = fmt.Sprintf("127.0.0.1:%d", grpcPort)
This approach ensures tests are resilient to parallel execution and prevents port conflicts in CI environments where multiple builds might run concurrently.
When writing tests for observability features like metrics, functions, and alerting expressions, ensure test cases cover practical, real-world monitoring scenarios in addition to edge cases. Focus on common usage patterns that developers will actually use in production monitoring and alerting.
For monitoring functions, design expressions that return meaningful results for alerting - avoid functions that return binary 0/1 values when you need to identify specific disappearing time series. Instead, create expressions that contain exactly the series you want to monitor.
Example of good practice:
# Test common usage pattern
eval range from 50s to 60s step 10s count_over_time(metric1_total[step()])
# For alerting on disappearing series, use expressions like:
present_over_time(some_metric[1h]) unless present_over_time(some_metric[5m])
This approach ensures your observability code works effectively in production monitoring scenarios, not just in isolated test conditions.
Choose appropriate data structures to minimize computational complexity when performing lookups. Prefer direct lookups (maps/dictionaries) over repeated array traversals, and use set-based inclusion checks for membership testing rather than multiple conditionals.
Instead of repeatedly traversing arrays:
// Inefficient - requires O(n) lookup each time
const integration = props.integrations.find(({key}) => key === value);
if (!integration) {
return true;
}
Consider:
// O(1) lookup after initial map creation
const integrationsByKey = new Map(props.integrations.map(i => [i.key, i]));
const integration = integrationsByKey.get(value);
// More maintainable and extensible approach: if (aggregation && NO_ARGUMENT_SPAN_AGGREGATES.includes(aggregation as AggregationKey)) {
This approach not only improves performance for large datasets but also makes code more maintainable when conditions need to be added or removed in the future.
---
## HTTP security headers
<!-- source: prometheus/prometheus | topic: Security | language: Markdown | updated: 2025-06-27 -->
Ensure that HTTP security headers are properly implemented and documented to prevent web vulnerabilities such as cache poisoning, clickjacking, and content type sniffing attacks. Security headers should be explicitly added to API responses and web endpoints as defensive measures.
When implementing security headers, consider headers like:
- `Vary: Origin` to prevent cache poisoning attacks
- `X-Content-Type-Options: nosniff` to prevent MIME type confusion
- `X-Frame-Options` or `Content-Security-Policy` to prevent clickjacking
- `Strict-Transport-Security` for HTTPS enforcement
Example from the codebase:
Vary: Origin header to responses to avoid cache poisoning. #16008
```Always document security-related changes clearly in changelogs and commit messages, specifying the vulnerability being addressed and the protective measure being implemented.
Always use descriptive variable, parameter, function, and constant names that clearly convey their purpose and behavior. Avoid single-letter variables or abbreviations except in well-established contexts like loop counters.
Why?
Examples:
❌ Poor naming:
const s = this.selection.value;
if (s) {
for (let c of this.state) {
if (c.source === s.source && c.index === s.index) {
this.selection.next(c);
break;
}
}
}
✅ Good naming:
const selection = this.selection.value;
if (selection) {
for (let connection of this.state) {
if (connection.source === selection.source && connection.index === selection.index) {
this.selection.next(connection);
break;
}
}
}
When naming similar functions, ensure the names clearly differentiate their behaviors:
// Names clearly indicate different purposes
shouldLoadPluginInFrontendSandbox() // Performs HTTP calls and checks
isPluginFrontendSandboxEligible() // Performs faster local checks
For UI elements and parameters that might conflict in shared namespaces, use prefixing conventions:
__returnToTitle) to avoid conflictsWhen temporarily versioning methods (e.g., with numeric suffixes), document when and how they will be renamed:
// TODO: Remove the '2' suffix after the feature flag is removed
applyLayoutStylesToDiv2() { ... }
Always clone arrays before performing operations that would mutate the original array, especially when working with component state or props. Direct mutations can cause subtle bugs and unexpected side effects in React applications.
Common mutating operations to avoid:
sort(), reverse(), or splice() directly on state arrayspush(), pop(), shift() or unshift() on existing arraysInstead, create a copy first:
// ❌ Avoid: This mutates the original array
messagePlaceholders.sort((a, b) => { ... });
// ✅ Better: Create a copy before sorting
messagePlaceholders.slice().sort((a, b) => { ... });
// or
[...messagePlaceholders].sort((a, b) => { ... });
For comparing sorted arrays:
// ❌ Avoid: This mutates both arrays during comparison
JSON.stringify(selectedLabels.sort()) !== JSON.stringify(prompt.labels.sort())
// ✅ Better: Create copies before sorting
JSON.stringify([...selectedLabels].sort()) !== JSON.stringify([...prompt.labels].sort())
When accessing array elements, prefer direct indexing over chained methods that create unnecessary intermediate arrays:
// ❌ Avoid: Creates an intermediate copy unnecessarily
widget.data.dimensions.slice().shift()?.field ?? "none";
// ✅ Better: Direct access is clearer and more efficient
widget.data.dimensions[0]?.field ?? "none";
Use appropriate null/undefined handling techniques based on the context to prevent runtime errors. Be mindful of the distinction between null and undefined in TypeScript, as they behave differently and affect type safety.
For accessing potentially undefined nested properties:
?.) for succinct null-safe property accessget() for complex paths or when you need a default valueWhen defining component props:
prop?: Type for truly optional props (results in undefined when not provided)prop: Type | null when the absence of a value needs explicit representation// AVOID: Unnecessary null checks when types guarantee non-null
title={title?.toString()} // Not needed if title is always defined
// AVOID: Unsafe property access without null checking
// @ts-ignore
acc[name] = config.custom.cellOptions.type; // May cause runtime errors
// BETTER: Use optional chaining or conditional checks
if (config?.custom?.cellOptions?.type) {
acc[name] = config.custom.cellOptions.type;
}
// BETTER: For complex paths, use lodash get with default values
const value = name ? get(formValues, name, '') : '';
Distinguish between unnecessary null checks (which add noise) and missing null checks (which cause bugs). Let TypeScript’s type system guide your null-handling needs while maintaining runtime safety.
Always leverage existing utility hooks, constants, and navigation methods instead of implementing manual solutions. This ensures consistency across the codebase, improves testability, and reduces maintenance overhead.
Key practices:
useUrlQuery instead of manual URLSearchParams handlingsafeNavigate instead of history.replace for proper browser navigation behaviorExample of preferred approach:
// Instead of manual URLSearchParams
const { search } = useLocation();
const searchParams = new URLSearchParams(search);
// Use existing utility
const { urlQuery, redirectWithQueryParams } = useUrlQuery();
// Instead of inline strings
searchParams.set('spanId', span.spanId);
// Use constants
searchParams.set(QueryParams.spanId, span.spanId);
// Instead of history.replace
history.replace({ search: searchParams.toString() });
// Use safeNavigate for better UX
safeNavigate({ spanId: span.spanId });
This approach promotes code reusability, ensures consistent behavior across components, and makes the codebase more maintainable by centralizing common API interaction patterns.
Performance optimizations should be validated with measurements rather than assumptions. When implementing performance-sensitive code or optimizations:
Example:
// Performance tested with:
// - 100k points: ~180ms initialization overhead
// - 10k points: ~20ms initialization overhead
// - 1k points: ~2ms initialization overhead
// Optimization is worthwhile for common use cases with >1k points
// Track coordinates to avoid rendering duplicate markers
const processedCoordinates = new Set<string>();
// Clean up resources when component unmounts
useEffect(() => {
return () => {
if (imageBlob) {
URL.revokeObjectURL(URL.createObjectURL(imageBlob));
}
};
}, [imageBlob]);
This approach ensures optimization efforts are focused on areas with measurable impact and helps prevent premature optimization that could introduce unnecessary complexity.
Distinguish between falsy values (0, ‘’, false, null, undefined) and null/undefined values in your code. Use explicit checks for null/undefined when you want to preserve other falsy values.
For absence checks, use:
// Instead of !value (catches all falsy values including 0, '', false)
if (value === undefined || value === null) { /* or simply: value == null */ }
For providing defaults, use the nullish coalescing operator:
// Instead of value || defaultValue (replaces all falsy values)
const result = value ?? defaultValue; // Only replaces null/undefined
For filtering collections, be specific about what you’re filtering:
// Instead of .filter(Boolean) (removes all falsy values)
array.filter(item => item != null); // Only removes null/undefined
When working with math operations on potentially empty collections:
// Provide a default to avoid -Infinity/Infinity
Math.max(0, ...array.map(item => item.value));
For browser APIs and nested objects:
// Check existence before accessing properties
const available = typeof window !== "undefined" && window.Feature?.isEnabled;
Use consistent, specific, and semantically appropriate naming conventions throughout the codebase. When creating new identifiers:
folder/${generateTimestamp()})For example:
// Instead of this:
builder.addRadio({
path: 'reduceOptions.sort',
settings: {
options: [
{ value: SortWithReducer.None, label: 'None' },
{ value: SortWithReducer.Asc, label: 'A-Z' },
{ value: SortWithReducer.Desc, label: 'Z-A' },
]
},
});
// Do this (for consistency with tooltip sorting):
builder.addRadio({
path: 'reduceOptions.sort',
settings: {
options: [
{ value: SortWithReducer.None, label: 'None' },
{ value: SortWithReducer.Asc, label: 'Ascending' },
{ value: SortWithReducer.Desc, label: 'Descending' },
]
},
});
This approach improves code readability, maintainability, and reduces cognitive load for developers navigating the codebase.
Always implement security features with fail-safe defaults that deny access or disable insecure functionality unless explicitly configured. When security-sensitive features must be available in development environments, add explicit environment checks to prevent their use in production.
For example, when allowing an insecure mode that might be needed during development:
// GOOD: Only allow insecure mode in development environments
if cfg.AllowInsecure && cfg.Env == setting.Dev {
// Enable insecure feature
}
// BAD: Allows insecure mode in any environment
if cfg.AllowInsecure {
// Enable insecure feature
}
Similarly, when implementing authorization logic:
This approach prevents accidental security bypasses in production while still enabling necessary development features in appropriate environments.
When implementing configurable features, ensure they degrade gracefully based on environment conditions like feature flags, plugin availability, or license status. Instead of showing disabled buttons or non-functional UI elements, provide informative feedback about why a feature is unavailable and how to enable it.
For feature flags:
{config.featureToggles.myFeature && (
<MyFeatureComponent />
)}
For optional dependencies:
{!config.rendererAvailable ? (
<Alert severity="info" title="Image renderer plugin not installed">
<Trans i18nKey="feature-availability.renderer-instructions">
To render images, you must install the{' '}
<a href="..." target="_blank" rel="noopener noreferrer">
Grafana image renderer plugin
</a>
. Please contact your administrator to install the plugin.
</Trans>
</Alert>
) : (
<FeatureControls />
)}
For edition-specific features:
{config.buildInfo.edition === GrafanaEdition.OpenSource && (
<OpenSourceSpecificContent />
)}
This approach improves user experience by setting clear expectations about feature availability and providing guidance for enablement, rather than exposing UI elements that won’t work in the current environment.
Identify and optimize frequently executed code paths by moving invariant calculations and conditional logic outside of loops, recursive functions, and other performance-critical sections. This reduces repeated evaluations and improves algorithm efficiency.
For example, instead of this:
if (query.sort) {
const collator = new Intl.Collator();
filtered = filtered.sort((a, b) => {
if (query.sort === 'alpha-asc') {
return collator.compare(a.title, b.title);
}
if (query.sort === 'alpha-desc') {
return collator.compare(b.title, a.title);
}
});
}
Prefer this:
if (query.sort) {
const collator = new Intl.Collator();
const mult = query.sort === 'alpha-desc' ? -1 : 1;
filtered = filtered.sort((a, b) => mult * collator.compare(a.title, b.title));
}
This principle applies broadly to:
When optimizing recursive algorithms, also ensure you have proper termination conditions to prevent infinite recursion, particularly when dealing with self-referential data structures.
When implementing algorithms, ensure they perform exactly the operations needed without introducing unintended side effects or limitations. Verify all calculations produce correct results and avoid assumptions that restrict applicability.
Three common mistakes to avoid:
// Incorrect: Arbitrary constant distorts the percentage
const percentage = 300 + (usage / MAX_EVENTS_FREE_PLAN) * 100;
// Correct: Calculate the actual percentage
const percentage = (usage / MAX_EVENTS_FREE_PLAN) * 100;
// Incorrect: Removes ALL 'metrics' segments
const segments = folder.filter(segment => segment !== 'metrics');
// Correct: Only removes 'metrics' if it's the last segment
const segments = folder[folder.length - 1] === 'metrics' ? folder.slice(0, -1) : folder;
// Incorrect: Forces minimum of 1, unsuitable for fractional values
const roundTo = Math.max(1, Math.pow(10, magnitude) / 5);
// Correct: Allows appropriate scaling for all value ranges
const roundTo = Math.pow(10, magnitude) / 5;
Ensure your algorithms precisely target the intended elements and handle all possible input values correctly.
When implementing observability with OpenTelemetry, place attributes at the appropriate level in the telemetry hierarchy to ensure effective data collection and analysis:
# Good: Static information as resource attributes
resource = Resource.create({
"environment": "production",
"service.name": "api-server",
"service.version": "1.0.0"
})
meter_provider = MeterProvider(resource=resource)
# Good: Valid baggage usage
processor = BaggageMeasurementProcessor(baggage_keys=["user.id", "synthetic_request"])
# Bad: Don't use trace.id in baggage; use exemplars instead for trace correlation
# Bad: Don't add timestamps to metrics
new_attributes["processed_at"] = str(int(time.time())) # Avoid this!
# Good: Use proper attribute fields in API calls
meter = meter_provider.get_meter("name", "version", schema_url="url", attributes={"component": "billing"})
This approach reduces cardinality issues, improves query performance, and follows OpenTelemetry best practices for attribute placement.
Design configuration options to be unambiguous and self-documenting. Avoid reusing the same configuration field for different data types, as this creates confusion and maintenance burden. Instead, use explicit indicators or separate fields to handle different scenarios. Make mutual exclusivity and constraints between configuration options explicit in both documentation and validation. Minimize duplication by designing unified approaches that work across similar use cases.
Example of problematic design:
# Avoid: Same field accepting different types
environment: <string|AzureCloudConfig>
Example of clearer design:
# Better: Use explicit indicator
environment: <string> # Use "custom" to enable custom_config
custom_config: <AzureCloudConfig> # Only used when environment="custom"
For mutually exclusive options, make the constraint explicit:
# Clear constraint documentation
promote_resource_attributes: [<string>, ...] # Cannot be used with promote_all_resource_attributes
promote_all_resource_attributes: <boolean> # Cannot be used with promote_resource_attributes
ignore_resource_attributes: [<string>, ...] # Only valid when promote_all_resource_attributes=true
Ensure observability implementations prioritize both mathematical correctness and code maintainability. Observability systems must produce accurate metrics and telemetry data while remaining easy to understand and modify.
For metric calculations, verify that aggregation operations are mathematically sound. Changing aggregation types (like countDistinct to count) requires careful consideration of their semantic differences and impact on data accuracy.
For telemetry tracking code, refactor complex logic into simpler, more maintainable structures. Consolidate duplicate tracking logic and use consistent patterns for event properties.
Example of maintaining correctness:
// Before: Incorrect aggregation change
AggregateOperator: v3.AggregateOperatorCount, // Wrong for distinct URLs
// After: Correct aggregation or proper alternative
AggregateOperator: v3.AggregateOperatorCountDistinct, // Correct for unique URLs
// OR use count with span_id if semantically equivalent
Example of improving maintainability:
// Before: Complex, duplicated telemetry logic with nested conditions
if alertMatched || dashboardMatched {
// 50+ lines of duplicated tracking logic
}
// After: Simplified, consolidated approach
queryInfoResult := NewQueryInfoResult(queryRangeParams, version)
aH.Signoz.Analytics.TrackUser(ctx, orgID, userID, "Telemetry Queried", queryInfoResult.ToMap())
Remember that correctness is non-negotiable in observability systems - users depend on accurate metrics for critical decisions about system health and performance.
Optimize algorithm structure by separating concerns and using appropriate data structures and language features. When implementing algorithms:
Separate search from processing to improve readability and maintainability. When you need to find and process an item, first locate the item, then apply operations to it rather than mixing these steps.
Use specialized data structures that match your algorithm’s needs instead of forcing operations into generic containers. Custom structures can combine related data for clearer intent and fewer traversals.
Leverage built-in operations like set intersections, comprehensions, or functional approaches instead of writing manual loops when possible.
Example of separating search from processing using functional approach:
# Instead of this:
for condition in evidence_data.conditions:
if condition["condition_result"] == target_priority:
threshold_type = fetch_threshold_type(Condition(condition["type"]))
resolve_threshold = fetch_resolve_threshold(condition["comparison"], group_status)
# More processing...
break
else:
raise ValueError("No threshold type found for metric issues")
# Do this:
try:
condition = next(
cond for cond in evidence_data.conditions
if cond["condition_result"] == target_priority
)
threshold_type = fetch_threshold_type(Condition(condition["type"]))
resolve_threshold = fetch_resolve_threshold(condition["comparison"], group_status)
# More processing...
except StopIteration:
raise ValueError("No threshold type found for metric issues")
Similarly, for set operations, use the built-in methods rather than explicit iteration:
# Instead of checking each element individually:
workflow_triggers = set()
for dcg in groups_to_fire[group.id]:
if dcg.id in trigger_group_to_dcg_model[DataConditionHandler.Group.WORKFLOW_TRIGGER]:
workflow_triggers.add(dcg)
# Do this:
workflow_triggers = groups_to_fire[group.id].intersection(
trigger_group_to_dcg_model[DataConditionHandler.Group.WORKFLOW_TRIGGER]
)
Always leverage established testing frameworks and APIs over custom bash scripts or direct Python execution for test automation. Using proper testing frameworks improves maintainability, provides better error handling, and enables more flexible test configuration.
For Python tests:
Example - Before:
python test.py http://127.0.0.1:5000/verify-tracecontext
Example - After:
export SERVICE_ENDPOINT=http://127.0.0.1:5000/verify-tracecontext
pytest test.py -k "not test_tracestate_duplicated_keys"
For test tooling scripts, prefer language-specific APIs over shell scripts for complex logic. This allows for better error handling and easier maintenance:
# Using a Python API for testing tools instead of bash
from griffe import check
def run_checks(branch="main"):
# More maintainable than equivalent bash
results = check.verify_against_branch(branch)
# Process results with proper error handling
Remember to document any test exclusions with clear explanations and FIXMEs for future follow-up.
Perform null and validity checks at the point where values are actually used, rather than defensively initializing or checking everywhere. This approach reduces unnecessary overhead while ensuring safety where it matters most.
When handling potentially null or invalid values:
Example from the codebase:
// Instead of defensive initialization:
if hints == nil {
hints = &storage.LabelHints{}
}
// Check at point of use:
func someFunction(hints *storage.LabelHints) {
if hints != nil {
// use hints safely
}
}
// Set to safe value on error:
var alertsByRule map[uint64][]*Alert
err = json.NewDecoder(file).Decode(&alertsByRule)
if err != nil {
alertsByRule = nil // Set to safe value
return
}
This pattern prevents unnecessary allocations, makes the code’s intent clearer, and ensures safety exactly where needed without over-engineering defensive checks throughout the codebase.
When designing or modifying APIs, ensure they follow established patterns and conventions within the codebase rather than introducing inconsistent approaches. This includes maintaining symmetrical interfaces, preserving backward compatibility when possible, and using consistent parameter types across similar functions.
Key principles:
Example from the codebase:
// Instead of exposing non-exported types in public APIs:
func (a *Annotations) Add(err annoErr) Annotations // ❌ Inconsistent
// Keep the established API and handle internally:
func (a *Annotations) Add(err error) Annotations { // ✅ Consistent
// Handle type assertions internally
}
This approach reduces friction for API consumers, maintains predictable interfaces, and ensures that related functionality follows similar patterns throughout the codebase.
Keep code clean and maintainable by removing unnecessary elements. This includes:
// Good const cell = renderTableHeadCell?.(column, _columnIndex);
2. Remove unnecessary Fragment wrappers when there's only one child:
```typescript
// Bad
<Fragment>
<WizardInstructionParagraph>
{content}
</WizardInstructionParagraph>
</Fragment>
// Good
<WizardInstructionParagraph>
{content}
</WizardInstructionParagraph>
<LoadingIndicator style={{margin: 0, marginTop: space(0.5)}} />
// Good
const StyledLoadingIndicator = styled(LoadingIndicator)
margin: ${space(0.5)} 0 0 0;
;
Following these practices improves code readability and maintains consistent patterns across the codebase.
---
## Centralize configuration management
<!-- source: SigNoz/signoz | topic: Configurations | language: Go | updated: 2025-06-20 -->
Consolidate configuration parameters into centralized config structures instead of using scattered options structs or hardcoded values. Configuration should be owned and controlled by the appropriate component, with all related settings grouped together in well-organized config files.
**Key principles:**
- Replace options structs with direct config parameter passing
- Move hardcoded values to configurable settings
- Group related configuration in single files that can handle multiple providers
- Ensure the controlling component manages its own configuration
**Example transformation:**
```go
// Before: Scattered options struct
type ServerOptions struct {
FluxInterval string
FluxIntervalForTraceDetail string
HTTPHostPort string
// ... many scattered options
}
func NewServer(serverOptions *ServerOptions) (*Server, error) {
// Complex parsing and handling
}
// After: Centralized config with direct parameters
func NewServer(config signoz.Config, signoz *signoz.SigNoz, jwt *authtypes.JWT) (*Server, error) {
// Direct access to organized config
reader := clickhouseReader.NewReader(
signoz.SQLStore,
signoz.TelemetryStore,
config.Querier.FluxInterval, // Controlled by the right component
// ...
)
}
This approach improves maintainability, reduces configuration drift, and ensures that components have proper ownership of their settings. Avoid hardcoded constants like timeout values or version strings - make them configurable through the centralized config system.
Move AI-specific functionality (especially prompts) to dedicated services rather than scattering them throughout the codebase. This improves maintainability and follows team conventions.
Specifically:
Example - Instead of:
def make_input_prompt(feedbacks):
feedbacks_string = "\n".join(f"- {msg}" for msg in feedbacks)
return f"""Task:
Instructions: You are an AI assistant that analyzes customer feedback.
Create a summary based on the user feedbacks...
"""
SUMMARY_REGEX = re.compile(r"Summary:\s*(.*)", re.DOTALL)
Prefer:
# In AI service module
def get_feedback_summary(feedbacks):
# Call centralized AI service with structured response format
response = ai_service.complete_prompt(
usecase="feedback_summary",
inputs={"feedbacks": feedbacks},
output_schema={"summary": str}
)
return response.summary
When adding common functionality like help targets, build rules, or other configuration elements that will be used across multiple projects or subdirectories, prefer centralizing them in shared configuration files rather than duplicating the implementation.
For Makefiles, use a common include file (like Makefile.common) that can be propagated across the organization. This approach reduces duplication, ensures consistency, and simplifies maintenance when changes are needed.
Example: Instead of adding the same help target to multiple Makefiles:
# In each subdirectory Makefile
help: ## Displays commands and their descriptions.
@grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}'
Centralize it in Makefile.common and include it:
# In subdirectory Makefiles
include ../../Makefile.common
This pattern applies to other configuration files as well - look for opportunities to extract common configuration elements into shared, includable files that can be maintained centrally and distributed across projects.
When writing Django migrations, avoid importing and referencing external models directly. Django migrations use auto-generated model classes based on the migration state at that point, not the current model definitions. These migration-specific models ensure compatibility with the database schema as it existed at that migration’s point in time.
Instead of importing models directly, use the apps.get_model() method provided in migration functions to access the correct version of a model:
# Bad practice:
from sentry.models.organization import Organization
def migrate_data(apps, schema_editor):
# Wrong: Organization here is the current model, not the migration state model
orgs = Organization.objects.filter(active=True)
# ...
# Good practice:
def migrate_data(apps, schema_editor):
# Correct: Gets the Organization model as it exists at migration time
Organization = apps.get_model("sentry", "Organization")
orgs = Organization.objects.filter(active=True)
# ...
This approach ensures migrations remain functional even as your models evolve over time, preventing subtle bugs that could corrupt data or break migration sequences.
Configure Celery tasks with acks_late=True when task reliability is critical and you prefer to risk duplicate processing rather than task loss. This setting ensures tasks aren’t acknowledged until successfully completed, allowing them to be reprocessed if a worker crashes or a deployment occurs during task execution.
Without this setting, if a worker is killed during task processing (e.g., during a deployment), the task will be lost as it was already acknowledged when received by the worker.
Example:
@instrumented_task(
name="sentry.replays.tasks.run_bulk_replay_delete_job",
queue="replays.delete_replay",
acks_late=True, # Ensure task durability during deployments
default_retry_delay=5,
max_retries=5,
silo_mode=SiloMode.REGION,
taskworker_config=TaskworkerConfig(namespace=replays_tasks, retry=Retry(times=5)),
)
def run_bulk_replay_delete_job(replay_delete_job_id: int, offset: int, limit: int = 100) -> None:
# Task implementation...
This is especially important for chained tasks, long-running operations, or tasks working with critical data where task loss would be more problematic than potential duplicate processing.
Ensure configuration options that are mutually exclusive are properly validated with clear, actionable error messages. When introducing configuration fields that cannot be used together, implement validation logic that checks for conflicts and provides specific guidance to users.
Key practices:
Example implementation:
func (c *OTLPConfig) UnmarshalYAML(unmarshal func(interface{}) error) error {
// ... unmarshaling logic ...
if c.PromoteAllResourceAttributes {
if len(c.PromoteResourceAttributes) > 0 {
return errors.New("'promote_all_resource_attributes' and 'promote_resource_attributes' cannot be configured simultaneously")
}
if err := validateAttributes(c.IgnoreResourceAttributes); err != nil {
return fmt.Errorf("invalid 'ignore_resource_attributes': %w", err)
}
} else {
if len(c.IgnoreResourceAttributes) > 0 {
return errors.New("'ignore_resource_attributes' cannot be configured unless 'promote_all_resource_attributes' is true")
}
}
return nil
}
This approach prevents configuration errors from causing runtime failures and helps users understand the correct configuration patterns.
Ensure proper metrics registration and lifecycle management to maintain observability system stability. Handle metric registration/unregistration correctly during configuration changes and avoid common pitfalls that can cause panics or data loss.
Key practices:
Use AlreadyRegisteredError pattern for shared metrics: When multiple components share the same metrics, handle registration conflicts gracefully by retrieving existing metrics from the registry instead of creating new ones.
Manage registration during config reloads: Properly unregister metrics before re-registering to avoid panics, but be aware this causes counter resets. Consider keeping metrics registered and reusing them when possible.
Register all required collectors: When using custom registries, ensure all necessary collectors (GoCollector, ProcessCollector) are registered, not just the default ones.
Example of proper AlreadyRegisteredError handling:
func newDiscovererMetrics(reg prometheus.Registerer) *zookeeperMetrics {
m := &zookeeperMetrics{
failureCounter: prometheus.NewCounter(prometheus.CounterOpts{
Name: "zookeeper_failures_total",
Help: "The total number of ZooKeeper failures.",
}),
}
// Handle shared metrics between components
if err := reg.Register(m.failureCounter); err != nil {
if are, ok := err.(prometheus.AlreadyRegisteredError); ok {
// Use the existing metric instead of the new one
m.failureCounter = are.ExistingCollector.(prometheus.Counter)
} else {
// Handle other registration errors
return nil
}
}
return m
}
This prevents registration panics while ensuring metrics remain functional across component lifecycles and configuration changes.
Adopt and document a consistent approach to API versioning across your services. The chosen format should be clearly communicated, whether using the simple form (e.g., "apiVersion": "v0alpha1") or the fully qualified form (e.g., "apiVersion": "prometheus.datasource.grafana.app/v0alpha1").
When exposing API version information:
// When defining API resources, be explicit about version requirements
type ResourceDefinition struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata"` // Required fields should not use omitempty
// If using OpenAPI generation, ensure annotations match expected behavior
// +optional
APIVersion string `json:"apiVersion,omitempty"`
// Other fields...
}
Clients should be designed with the understanding that API resources may expose multiple versions, and avoid making assumptions about specific version formats. This promotes API stability and enables easier version transitions.
Always raise specific, contextual exception types instead of generic exceptions. Include relevant error details that help identify the root cause and potential solutions. This improves error tracking, debugging, and system reliability.
Bad:
if response.status_code != 200:
raise Exception("A non 200 HTTP status code was returned.")
if not data:
raise Exception("Invalid response from LLM")
Good:
if response.status_code != 200:
raise ValueError(
f"Seer API returned unexpected status code {response.status_code}: {response.text}"
)
if not data:
raise ValueError("LLM returned empty response when generating summary")
The specific exception type (e.g., ValueError, TypeError, KeyError) should match the error condition. Include relevant context like parameter values, system state, or error messages in the exception text. This helps with:
Excessive indentation makes code harder to read and understand. Two common techniques can help flatten your code structure:
# Instead of this:
def increment_group_tombstone_hit_counter(tombstone_id: int | None, event: Event) -> None:
if tombstone_id:
# deeply nested logic here
# ...
# Prefer this:
def increment_group_tombstone_hit_counter(tombstone_id: int | None, event: Event) -> None:
if not tombstone_id:
return
# main logic here at the top level
# ...
# Instead of this:
with mock.patch(...):
with self.feature(...):
with self.tasks():
with pytest.raises(...):
# deeply nested code
# Prefer this:
with (
mock.patch(...),
self.feature(...),
self.tasks(),
pytest.raises(...)
):
# code at a more readable indentation level
These techniques produce flatter, more scannable code that’s easier to maintain and debug. This pattern is sometimes called “flattening arrow code” as it helps avoid the rightward drift that can make functions hard to follow.
Variable and function names should clearly communicate their actual purpose and behavior, not just their initial intent. Avoid vague or misleading names that don’t accurately reflect what the code does.
When naming variables and functions, consider:
Example of problematic naming:
// Misleading - suggests it's only used for initialization
const initialValue = getDefaultValue();
// Better - clearly indicates it's used for resets
const defaultValue = getDefaultValue();
// Vague - doesn't explain what data type operation this performs
export const getDataType = (dataType?: string): DataTypes => {
// implementation
}
// Better - specific about the conversion/mapping being performed
const mapStringToDataType = (dataType?: string): DataTypes => {
// implementation
}
Names should serve as documentation for future maintainers, making the code’s intent immediately clear without requiring deep analysis of the implementation.
Prioritize memory allocation minimization through buffer reuse, pre-allocation, and efficient data structures. Allocations are among the worst performance bottlenecks, especially in hot paths and frequently called functions.
Key strategies:
make([]T, len) instead of make([]T, 0, len) with append() when the final size is knownstruct{} instead of bool for existence-only maps, and prefer smaller integer types when ranges allowExample of buffer reuse:
// Instead of creating new slices
to.NegativeSpans = nil
to.NegativeBuckets = nil
// Reuse existing capacity
if to.NegativeSpans != nil {
to.NegativeSpans = to.NegativeSpans[:0]
}
if to.NegativeBuckets != nil {
to.NegativeBuckets = to.NegativeBuckets[:0]
}
Example of pre-allocation:
// Instead of append() with unknown final size
vs.Series = make([]storage.Series, 0, len(mat))
for _, s := range mat {
vs.Series = append(vs.Series, NewStorageSeries(s))
}
// Pre-allocate and index directly
vs.Series = make([]storage.Series, len(mat))
for i, s := range mat {
vs.Series[i] = NewStorageSeries(s)
}
Always profile allocation-heavy code paths and consider the memory lifecycle of objects, especially in high-frequency operations like query evaluation, scraping, and data ingestion.
Use consistent and descriptive names for variables, components, and functions throughout the codebase. When the same concept appears in multiple places, use identical naming to prevent confusion and bugs. This includes:
// Inconsistent (confusing):
const modelToProviderMap = useMemo(() => {
const modelProviderMap: Record<string, string> = {}; // Different name!
// ...
}, [apiKeys.data]);
// Consistent (clear):
const modelToProviderMap = useMemo(() => {
const modelToProviderMap: Record<string, string> = {}; // Same name
// ...
}, [apiKeys.data]);
// Misleading (no longer a dropdown):
export function DownloadDropdown({ /* ... */ }) {
// Renders a single button, not a dropdown
}
// Clear (matches functionality):
export function DownloadButton({ /* ... */ }) {
// Renders a single button
}
// Less clear:
const autoLocked = useState<boolean>(isExistingWidget);
// More clear:
const isAutoLocked = useState<boolean>(isExistingWidget);
// Inconsistent:
totalTokens: props.observations.map(/* ... */)
// Consistent with other code:
totalUsage: props.observations.map(/* ... */)
// Incorrect:
export const PeakViewTraceDetail = ({ projectId }: { projectId: string }) => {
// Correct (matches file path and context):
export const PeekViewTraceDetail = ({ projectId }: { projectId: string }) => {
Consistent naming reduces cognitive load for developers, makes the codebase more maintainable, and helps prevent bugs caused by name confusion.
Always add rel="noopener noreferrer" to external links that use target="_blank" to prevent tabnabbing attacks. This security attribute prevents malicious websites from gaining access to your window object through the opener property, which could be exploited for phishing attacks or other security breaches.
Example:
// Insecure: vulnerable to tabnabbing
<Link
href="https://langfuse.com/docs/analytics/posthog"
target="_blank"
>
Integration Docs ↗
</Link>
// Secure: protected against tabnabbing
<Link
href="https://langfuse.com/docs/analytics/posthog"
target="_blank"
rel="noopener noreferrer"
>
Integration Docs ↗
</Link>
This security measure should be applied consistently across all external links in the application, especially in components that render user-provided or dynamic URLs.
When implementing functionality that processes multiple items, always consider the performance implications of individual versus batch processing. Avoid fetching or processing items one-by-one in loops, as this can lead to significant performance degradation, especially at scale. Instead:
Example - Problematic:
# Performance issue: Individual queries in a loop
def fetch_error_details(project_id: int, error_ids: list[str]) -> list[dict[str, Any]]:
error_details = []
for error_id in error_ids:
try:
event = eventstore.get_event_by_id(project_id, error_id) # Separate query for each ID
# Process event...
# ...
return error_details
Example - Improved:
# Better: Single bulk query
def fetch_error_details(project_id: int, error_ids: list[str]) -> list[dict[str, Any]]:
if not error_ids:
return []
# Use a bulk query instead of individual queries
events = eventstore.get_events_by_ids(project_id, error_ids)
# Process events...
return error_details
This pattern applies to any situation where you might be tempted to make repeated database calls, API requests, or process large amounts of data item-by-item. Bulk operations significantly reduce overhead, network latency, and processing time.
Log messages should be clear, unambiguous, and avoid creating confusion for developers and operators. This means explicitly logging when no-op operations occur (rather than staying silent) and avoiding duplicate log entries with identical information that can mislead readers.
Silent operations in dependency injection scenarios can be particularly dangerous as they hide important system behavior. Always add explicit logging to indicate when no-op implementations are being called.
Similarly, avoid logging the same information multiple times with identical fields, as this creates confusion about whether multiple events occurred or if there’s a logging bug.
Example of good practice:
func (provider *provider) Send(ctx context.Context, messages ...analyticstypes.Message) {
// Explicitly log the no-op behavior
provider.logger.Debug("noop analytics provider called - no messages will be sent")
}
// Instead of logging both error and info with same fields:
if err != nil {
fields = append(fields, zap.Error(err))
middleware.logger.Error(logMessage, fields...)
return // Don't also log info
}
middleware.logger.Info(logMessage, fields...)
Always use sanitized navigation utilities instead of directly manipulating window.location with potentially user-influenced data to prevent cross-site scripting (XSS) vulnerabilities. User-provided values can contain malicious scripts that execute when inserted into navigation contexts.
Bad:
window.location.assign(this.newPath); // Dangerous if newPath contains user input
Good:
testableWindowLocation.assign(this.newPath); // Uses a wrapper that sanitizes inputs
Use wrapper functions or utilities that perform proper validation and sanitization of URLs before navigation. This approach not only improves security but also makes testing easier by providing a mockable interface for navigation operations.
When documenting test tools, migration utilities, or testing frameworks, provide comprehensive explanations of different modes or options available, including their trade-offs and appropriate use cases. This helps developers make informed decisions about which approach best fits their testing needs.
Key elements to include:
For example, when documenting a test migration tool:
The `--mode` flag controls how expectations are migrated:
- `strict`: Strictly migrates all expectations to the new syntax.
This is probably more verbose than intended because the old syntax
implied many constraints that are often not needed.
- `basic`: Like `strict` but never creates `no_info` and `no_warn`
expectations. This can be a good starting point to manually add
`no_info` and `no_warn` expectations and/or remove `info` and
`warn` expectations as needed.
- `tolerant`: Only creates `expect fail` and `expect ordered` where
appropriate. All desired expectations about presence or absence
of `info` and `warn` have to be added manually.
All three modes create valid passing tests from previously passing tests.
`basic` and `tolerant` just test fewer expectations than the previous tests.
This approach prevents confusion and helps teams choose the right testing strategy for their specific context.
Maintain consistency in configuration values across the application by following these practices:
Use properly formatted template strings for configuration keys Template strings for configuration keys should be properly formatted with correct syntax to avoid runtime errors and ensure consistent storage.
// Incorrect
const key = `${projectId}-${datasetId-chart-metrics`;
// Correct
const key = `${projectId}-${datasetId}-chart-metrics`;
Update identifiers when functionality changes When updating functionality that relies on persistent identifiers (like notifications), update the identifier to ensure proper behavior for all users.
// When updating a notification:
// Old
id: "lw3-1",
// New (when content changes)
id: "lw3-2",
Use consistent default values for the same parameter When a parameter is used in multiple places, ensure that default values are consistent to avoid confusing behavior.
// Inconsistent defaults
const [selectedTab] = useQueryParam("display", withDefault(StringParam, "details"));
// vs elsewhere
onExpand={() => setSelectedTab("preview")}
// Consistent defaults
const [selectedTab] = useQueryParam("display", withDefault(StringParam, "preview"));
Avoid hardcoded values in favor of configurable settings Configuration values (especially URLs, endpoints, and service-specific settings) should be configurable rather than hardcoded, allowing for easier environment-specific customization.
// Hardcoded (avoid)
case LLMAdapter.Atla:
return "https://api.atla-ai.com/v1/integrations/langfuse";
// Configurable (preferred)
case LLMAdapter.Atla:
return customization?.defaultBaseUrlAtla ?? "";
Tests should be independent and isolated from each other to prevent state leakage and ensure reliable results. When testing components that maintain state (like singletons) or require specific environments (like PHP version-specific features), choose appropriate isolation strategies:
// In your test file: Php81/YourTest.php /**
Failing to maintain test isolation can lead to inconsistent results, false positives/negatives, and tests that pass or fail based on execution order rather than correctness.
When implementing caching solutions, clearly document and justify your caching strategy decisions based on access patterns, performance requirements, and complexity trade-offs. Consider whether the benefits of complex caching approaches outweigh simpler alternatives.
For cache duration decisions, align TTL with content characteristics - static assets can use longer durations (days/weeks), while dynamic data may need shorter periods. For cache loading strategies, evaluate whether preloading provides sufficient performance benefits over on-demand caching to justify the additional complexity.
Example from the discussions:
// Good: Justified preload strategy with clear reasoning
func (r *ClickHouseReader) PreloadMetricsMetadata(ctx context.Context, orgID valuer.UUID) ([]string, *model.ApiError) {
// Preload all metrics metadata to avoid redundant ClickHouse queries
// for each metric name lookup, trading memory for query performance
// Good: Appropriate cache duration for static web assets
cache := middleware.NewCache(7 * 24 * time.Hour) // Static builds don't change
}
Always question whether additional cache keys or complex strategies provide measurable benefits over simpler approaches. Document the reasoning behind your caching decisions to help future maintainers understand the trade-offs made.
Documentation comments should be accurate, concise, and add genuine value rather than restating obvious code behavior. Focus on explaining “what” the code does and “why” it exists, not “how” it works step-by-step.
Key principles:
Example of good documentation:
// addTypeAndUnitLabels appends type and unit labels to the given labels slice.
func addTypeAndUnitLabels(labels []prompb.Label, metricType, unit string) []prompb.Label {
// implementation
}
Example of poor documentation:
// addTypeAndUnitLabels appends type and unit labels to the given labels slice if the setting is enabled.
// This function iterates through the labels and adds new ones based on the parameters provided.
func addTypeAndUnitLabels(labels []prompb.Label, metricType, unit string) []prompb.Label {
// implementation that doesn't actually check any setting
}
Assume readers are proficient in Go and focus documentation on the purpose and behavior rather than implementation details.
Always implement multiple layers of security controls, even when they might seem redundant. This strategy ensures protection remains effective even if one security measure fails or is bypassed.
For external links:
// Always add rel="noopener" to links with target="_blank"
DOMPurify.addHook('afterSanitizeAttributes', function (node) {
if (node.tagName === 'A' && node.getAttribute('target') === '_blank') {
const currentRel = node.getAttribute('rel') || '';
const relValues = new Set(currentRel.split(/\s+/).filter(Boolean));
relValues.add('noopener');
// Apply even if browsers may handle this implicitly
}
});
For input validation:
// Be thorough when escaping special characters
function escapeInput(value: string): string {
// Apply comprehensive escaping that covers all edge cases
return typeof value === 'string'
? value.replace(/\\/g, '\\\\\\\\').replace(/[$^*{}\[\]'+?.()|\/]/g, '\\$&')
: value;
}
This principle applies to all security contexts: authentication, authorization, input validation, and output encoding. By incorporating overlapping protections, you create a more robust security posture that doesn’t rely on any single control being perfect.
Always analyze shared data access patterns to prevent race conditions in concurrent code. Use appropriate synchronization mechanisms based on access patterns and data ownership.
Key practices:
Use RLock for read-heavy scenarios: When data is read frequently but written rarely, use sync.RWMutex with RLock() to allow concurrent reads while still protecting against concurrent writes.
Copy data when sharing between goroutines: When the same data structure will be accessed by multiple goroutines that might modify it, create copies to prevent races.
Avoid unmanaged goroutines: Don’t spawn goroutines without proper lifecycle management. Use sync.WaitGroup or context cancellation to ensure cleanup.
Understand library thread safety: Before adding locks around external library calls, verify if the library is already thread-safe. Adding unnecessary locks can cause performance issues.
Example of proper data copying to prevent races:
func relabelAlerts(alerts []*Alert) []*Alert {
var relabeledAlerts []*Alert
for _, s := range alerts {
// Copy the alert to avoid race condition when multiple
// goroutines modify the same alert pointers
a := s.Copy()
// ... relabel operations on copy
relabeledAlerts = append(relabeledAlerts, a)
}
return relabeledAlerts
}
Always run tests with -race flag to detect potential race conditions during development.
Make authentication and security-related boolean conditions explicit and direct rather than using intermediate variables that obscure the logic. This improves code readability, reduces the risk of misconfiguration, and makes security decisions more auditable.
When configuring authentication settings, express the condition directly in the assignment rather than using intermediate boolean variables that add unnecessary complexity.
Example:
// Instead of:
noAuth := true
if conf.ServiceAccountKey != "" || conf.ServiceAccountKeyPath != "" {
noAuth = false
}
config := &Configuration{
NoAuth: noAuth,
}
// Use direct boolean expression:
config := &Configuration{
NoAuth: conf.ServiceAccountKey == "" && conf.ServiceAccountKeyPath == "",
}
This pattern makes the authentication logic immediately clear to reviewers and reduces the chance of introducing bugs through incorrect intermediate variable handling.
When modifying existing APIs, implement changes that maintain backward compatibility while enabling new functionality. For public-facing APIs, avoid breaking changes outside of major version bumps.
Key strategies:
func_get_args() for parameters that would break signatures@deprecated and trigger deprecation notices@internal annotations for components not meant for public consumption// AVOID (breaking change):
public function startTransaction(TransactionContext $context, array $customSamplingContext): Transaction;
// BETTER (backward compatible):
public function startTransaction(TransactionContext $context, ?array $customSamplingContext = null): Transaction;
// OR use implementation tricks:
public function startTransaction(TransactionContext $context): Transaction
{
// Handle optional parameter without changing signature
$customSamplingContext = func_num_args() > 1 ? func_get_arg(1) : null;
// Implementation
}
// For deprecating methods:
/**
* @deprecated since version 2.2, to be removed in 3.0
*/
public function legacyMethod(): void
{
@trigger_error(sprintf('Method %s() is deprecated since version 2.2 and will be removed in 3.0.', __METHOD__), E_USER_DEPRECATED);
// Call new implementation or keep legacy code
}
When implementing database schema changes, follow these migration best practices to maintain data integrity and ensure backward compatibility:
-- DO NOT add new columns to existing migration files
-- packages/shared/prisma/migrations/20230924232619_datasets_init/migration.sql
CREATE TABLE "dataset_items" (
/* existing fields */
"comment" TEXT, -- ❌ Adding here breaks compatibility
);
-- DO create a separate migration file for new columns
-- packages/shared/prisma/migrations/20250607040909_add_comment_to_dataset_items/migration.sql
ALTER TABLE "dataset_items" ADD COLUMN "comment" TEXT; -- ✓ Correct approach
Use consistent naming conventions for migration files to ensure correct execution order. For this project, use 4 leading digits in filenames (e.g., ‘0011’ not ‘00011’).
Use database-specific syntax correctly. For example, with ClickHouse:
ON CLUSTER default for cluster operationsDROP MATERIALIZED VIEW not DROP VIEW)Following these practices ensures migrations run reliably across all environments and prevents data inconsistencies.
Design APIs that can evolve without breaking existing code. Use keyword-only arguments (with *,) for optional parameters to make API extensions safer. This prevents positional argument errors when parameters change and allows for easier parameter reordering, addition, and deprecation.
# Problematic: Adding parameters later can break existing callers
def create_counter(self, name: str, unit: str = "", description: str = ""):
pass
# Better: Force keyword arguments for optional parameters
def create_counter(self, name: str, *, unit: str = "", description: str = "",
advisory: Optional[MetricsCommonAdvisory] = None):
pass
For types that may evolve structurally, use more generic container types or provide extension mechanisms. When designing class hierarchies, consider how derived classes will override methods - match parameter names and types carefully to avoid incompatibilities.
When introducing new functionality beyond the specification, implement it as utility functions rather than extending core interfaces. This prevents application code from becoming tightly coupled to specific implementations:
# Avoid adding non-spec methods directly to SDK classes
class Histogram(_Synchronous, APIHistogram):
def time(self): # Problematic: Application code depends on SDK implementation
return _Timer(self)
# Better: Create utility functions
def get_timer(histogram): # Good: Application code uses utility function
return _Timer(histogram)
This approach maintains clear boundaries between API contracts and implementation details while providing flexibility for future evolution.
Explicitly document all input and output constraints directly in API definitions. For inputs, specify validation rules such as length limits, character restrictions, and value ranges. For outputs, detail the exact format of returned data structures, including array ordering, boundary conditions (inclusive/exclusive), and semantic meanings of values. This practice improves API clarity and reduces integration errors.
Example (for API responses):
metrics:
properties:
data:
type: array<array<number>>
docs: |
Histograms will return an array with [lower, upper, height] tuples,
where lower is inclusive, upper is exclusive, and bins are sorted by lower bound.
Example (for API requests):
request:
body:
properties:
name:
type: string
docs: Project name (must be between 3-30 characters)
Ensure React hooks and state updates follow best practices to prevent subtle bugs:
// INCORRECT
function Component() {
const someCondition = true;
if (someCondition) {
useEffect(() => {/* effect */}, []); // Violation of hook rules
}
const memoizedData = useMemo(() => {
const { getNavigationPath } = useTracePeekNavigation(); // Hooks inside other hooks
return { getNavigationPath };
}, []);
}
// CORRECT
function Component() {
const { getNavigationPath } = useTracePeekNavigation();
const memoizedData = useMemo(() => {
return { getNavigationPath }; // Use hook result inside memo
}, [getNavigationPath]);
}
// INCORRECT - missing 'canEdit' dependency
const handleWidthChange = useCallback(
(containerWidth) => {
setEditable(canEdit && cols === 12); // Uses canEdit but missing from deps
},
[cols]
);
// CORRECT
const handleWidthChange = useCallback(
(containerWidth) => {
setEditable(canEdit && cols === 12);
},
[cols, canEdit] // All dependencies listed
);
// INCORRECT - may use stale state
setOpen((value) => {
const newValue = typeof value === "function" ? value(open) : value;
// Using 'open' can be stale
});
// CORRECT - always uses fresh state
setOpen((currentOpen) => {
const newValue = typeof value === "function" ? value(currentOpen) : value;
return newValue;
});
// INCORRECT - not reactive
disabled={isInProgress.data || !form.getValues().targetId}
// CORRECT - reacts to field changes
disabled={isInProgress.data || !form.watch('targetId')}
Following these patterns helps prevent difficult-to-debug issues like inconsistent renders, stale closures, and unpredictable component behavior.
Cache the results of expensive operations and component renders to avoid redundant calculations during re-renders. This improves application performance, especially when working with large datasets or complex transformations.
Key practices:
React.useMemo() for derived data transformations:
```jsx
// Instead of this:
const renderedData = data.map((item) => {
// expensive transformation
});// Do this: const renderedData = React.useMemo(() => { return data.map((item) => { // expensive transformation }); }, [data]);
2. Implement complete comparison functions in `React.memo()`:
```jsx
// Incorrect - incomplete prop comparison
const MemoComponent = React.memo(Component, (prev, next) => {
return prev.id === next.id; // Missing comparison of other props
});
// Correct - comprehensive comparison
const MemoComponent = React.memo(Component, (prev, next) => {
return prev.id === next.id && prev.data === next.data;
});
// Parse once and reuse: const parsedData = JSON.parse(rawData); const filteredItems = items.filter(item => parsedData.id === item.id);
4. Pre-compute values used in filtering operations:
```jsx
// Instead of mapping inside filter:
const validScores = scores.filter(scoreId =>
allScores.data?.scores.map(s => s.id).includes(scoreId)
);
// Pre-compute the lookup list:
const scoreIdSet = new Set(allScores.data?.scores.map(s => s.id));
const validScores = scores.filter(scoreId => scoreIdSet.has(scoreId));
Proper memoization prevents unnecessary recalculations and renders, significantly improving performance for data-intensive applications.
When configuring dependency management tools like Renovate, ensure comprehensive setup that includes modern syntax, strategic dependency grouping, and controlled update policies. Use current configuration formats (e.g., cron syntax instead of deprecated natural language), implement proper dependency grouping strategies, and configure update separation for better control.
Key practices:
"schedule": ["0 0-8 * * 1"], "timezone": "UTC" instead of deprecated natural language"separateMajorMinor": true, "separateMultipleMajor": true to maintain compatibility controlExample configuration:
{
"schedule": ["0 0-8 * * 1"],
"timezone": "UTC",
"separateMajorMinor": true,
"separateMultipleMajor": true,
"packageRules": [
{
"description": "Group related UI dependencies",
"matchPaths": ["web/ui/**"],
"groupName": "UI Dependencies"
}
]
}
This ensures dependency management is predictable, maintainable, and aligned with project requirements while leveraging the full capabilities of the tooling.
Use explicit null/undefined checks instead of truthy/falsy checks when handling potentially null or undefined values. Relying on truthy/falsy checks can lead to bugs when dealing with valid falsy values like 0 or empty strings.
Bad:
// May mishandle valid 0 values
if (curr.outputCost) {
total = total.plus(curr.outputCost);
}
// Could yield "undefined" string
const username = String(env.USERNAME);
Good:
// Explicitly checks for null/undefined
if (curr.outputCost != null) {
total = total.plus(curr.outputCost);
}
// Uses nullish coalescing for default
const username = env.USERNAME ?? "default";
When parsing JSON or handling unknown data:
Before creating new utility functions, check if existing ones can be reused or extended. Avoid code duplication by leveraging established utilities and creating single sources of truth for repeated patterns.
When you encounter repeated conditional logic or similar functionality across multiple places, consolidate it into a centralized solution. For example, instead of duplicating conditional assignments based on feature flags throughout the codebase, create a mapping object or utility function that serves as the single source of truth.
Example of what to avoid:
// Multiple places with similar conditional logic
aggregateAttribute: dotMetricsEnabled
? 'kafka.consumer.group.lag'
: 'kafka_consumer_group_lag'
// In another file
metricName: dotMetricsEnabled
? 'kafka.producer.records'
: 'kafka_producer_records'
Example of preferred approach:
// Create a centralized mapping
const getMetricKey = (baseKey: string, dotMetricsEnabled: boolean) => {
const metricKeyMap = {
'kafka_consumer_group_lag': 'kafka.consumer.group.lag',
'kafka_producer_records': 'kafka.producer.records'
};
return dotMetricsEnabled ? metricKeyMap[baseKey] : baseKey;
};
// Use existing formatting utilities instead of creating new ones
const formattedValue = getYAxisFormattedValue(value, format);
This approach improves maintainability, reduces the risk of inconsistencies, and makes the codebase easier to understand and modify.
Always use the standardized authtypes.ClaimsFromContext method for extracting authentication claims from request context, and handle errors properly. This ensures consistent authentication handling across the codebase and prevents potential security vulnerabilities from incorrect claim extraction.
The correct pattern is:
claims, err := authtypes.ClaimsFromContext(r.Context())
if err != nil {
render.Error(rw, err)
return
}
Avoid custom claim extraction methods or incorrect error handling patterns like claims, ok := authtypes.ClaimsFromContext(r.Context()) followed by if ok != nil - this can lead to authentication bypasses if the error handling logic is inverted. Using the standardized method ensures proper error propagation and consistent security behavior across all handlers.
When implementing common algorithmic tasks like string parsing, sorting, or comparison operations, prefer built-in utilities and standard library functions over custom implementations. Standard algorithms are typically more efficient, well-tested, and maintainable than custom solutions.
For string parsing, use built-in shell features:
# Instead of multiple parameter expansions:
major=${current_full_version%%.*}
minor=${current_full_version#*.}; minor=${minor%%.*}
# Use single-line built-in parsing:
IFS='.' read -r major minor _ <<< "$(go mod edit -json go.mod | jq -r .Go)"
For version comparison, leverage system utilities:
# Instead of custom compare_versions() function:
if ! echo -e "${min_version}\n${version}" | sort --version-sort --check=silent; then
# handle version mismatch
fi
This approach reduces code complexity, improves reliability, and often provides better performance than reinventing common algorithmic patterns.
All user-controlled inputs must be validated and sanitized before use to prevent injection attacks, unauthorized access, and other security vulnerabilities. This includes query parameters, environment variables, URLs, and database values.
When handling user inputs:
// Validate URL parameters
baseUrl: z.string().url(),
// Validate security-related environment variables
LANGFUSE_S3_EVENT_UPLOAD_SSE: z.enum(["AES256", "aws:kms"]).optional(),
// INSECURE: Using string interpolation
Prisma.sql`AND ${model} ~ match_pattern`
// SECURE: Using parameterized queries
Prisma.sql`AND $1 ~ match_pattern`, model
// Validate query parameters
if (!id || typeof id !== 'string') {
return res.status(400).json({ error: 'Valid ID required' });
}
Proper input validation is your first line of defense against many common security vulnerabilities including SQL injection, XSS, CSRF, and path traversal attacks.
Maintain consistent naming patterns across all database objects. Ensure that:
Example of good practice:
CREATE TABLE "default_llm_models" (
"id" TEXT NOT NULL,
"project_id" TEXT NOT NULL,
-- other columns...
);
-- Index name matches table name
CREATE INDEX "default_llm_models_project_id_id_key" ON "default_llm_models"("project_id", "id");
Example of consistent column formatting:
CREATE TABLE blob_storage_file_log
(
`id` String,
`project_id` String,
-- All columns use consistent backtick formatting
`event_ts` DateTime64(3),
`is_deleted` UInt8
);
Consistent naming patterns improve readability, reduce errors, and make database schemas more maintainable.
Always define explicit and minimal permission boundaries in GitHub Actions workflows to adhere to the principle of least privilege. By default, workflows have broad permissions that could be exploited if compromised. Specify permissions at either the workflow or job level to restrict the GITHUB_TOKEN to only required access levels.
Example:
name: Example Workflow
# Recommended: Set permissions at workflow level
permissions:
# Start with minimal permissions
contents: read
# Add other permissions only as needed:
# issues: write
# pull-requests: write
jobs:
example-job:
runs-on: ubuntu-latest
# Alternatively, set permissions at job level
# permissions:
# contents: read
steps:
- uses: actions/checkout@v4
# Job steps
This approach prevents potential security breaches by limiting the access scope of workflow runs, reducing the impact of supply chain attacks or compromised dependencies.
Always use semantically correct error types and codes that match the actual failure scenario. This ensures proper error handling by callers and consistent API behavior across the system.
Key Guidelines:
TypeNotFound/CodeNotFound for missing resources, not TypeInternal/CodeInternalTypeInvalidInput/CodeInvalidInput for validation failures, not TypeForbidden/CodeForbiddenExamples:
❌ Incorrect - Wrong error type for not found:
return nil, s.sqlstore.WrapNotFoundErrf(err, errors.CodeInternal, "unable to fetch the license with ID: %s", licenseID)
✅ Correct - Proper not found error:
return nil, s.sqlstore.WrapNotFoundErrf(err, errors.CodeNotFound, "unable to fetch the license with ID: %s", licenseID)
❌ Incorrect - Wrong error type for validation:
return errors.Wrapf(nil, errors.TypeForbidden, errors.CodeForbidden, "s3 buckets can only be added to service-type[%s]", services.S3Sync)
✅ Correct - Proper validation error:
return errors.Newf(errors.TypeInvalidInput, errors.CodeInvalidInput, "s3 buckets can only be added to service-type[%s]", services.S3Sync)
❌ Incorrect - Missing error check:
tracesJSON, _ := json.Marshal(tracesFilters)
✅ Correct - Always check errors:
tracesJSON, err := json.Marshal(tracesFilters)
if err != nil {
return nil, err
}
This practice ensures that error handling is predictable and allows callers to make appropriate decisions based on the specific type of failure.
Always validate bounds, nil pointers, and other potentially unsafe conditions before accessing values to prevent runtime panics and undefined behavior. This includes checking array/slice bounds, validating pointer references, and ensuring data structures are in expected states before use.
Key validation patterns:
Example from bounds checking:
size := ts.Size()
for i := start; i <= stop && i >= 0 && i < size; i++ {
if tok := ts.Get(i); tok != nil && tok.GetTokenType() != antlr.TokenEOF {
b.WriteString(tok.GetText())
}
}
Example from pointer validation:
func ValidatePointer(dest any, caller string) error {
rv := reflect.ValueOf(dest)
if rv.Kind() != reflect.Pointer || rv.IsNil() {
return WrapCacheableErrors(reflect.TypeOf(dest), caller)
}
return nil
}
This proactive validation approach prevents common sources of runtime errors and makes code more robust by catching potential issues before they cause panics or undefined behavior.
Always provide user-visible feedback for errors instead of only logging to the console. This ensures users are aware of failures and can take appropriate action. This is especially important for:
Example converting console-only error to user feedback:
// Before
const handleAction = async () => {
try {
await someAsyncOperation();
} catch (err) {
console.error(err); // Silent failure
}
};
// After
const handleAction = async () => {
try {
await someAsyncOperation();
} catch (err) {
console.error(err);
showErrorToast("Operation failed", err.message);
// or
setError("Failed to complete action. Please try again.");
}
};
For mutations, implement both success and error handlers:
const mutation = api.something.mutate.useMutation({
onSuccess: () => {
showSuccessToast("Operation completed");
// Update UI state
},
onError: (e) => {
showErrorToast("Operation failed", e.message);
// Preserve UI state
}
});
Always pin external dependencies to specific versions in CI/CD workflows to ensure build reproducibility, stability, and security. This applies to:
For GitHub Actions:
# ❌ Avoid:
- uses: pierotofy/set-swap-space@master
- uses: docker/login-action@65b78e6e13532edd9afa3aa52ac7964289d1a9c1
# ✅ Prefer:
- uses: pierotofy/set-swap-space@v1.2.0
- uses: docker/login-action@v2
For external tools:
# ❌ Avoid:
curl -sSLo shfmt https://github.com/mvdan/sh/releases/latest/download/shfmt_linux_amd64
# ✅ Prefer:
curl -sSLo shfmt https://github.com/mvdan/sh/releases/download/v3.7.0/shfmt_linux_amd64
For runtime environments:
# ❌ Avoid:
node-version: [20]
# ✅ Prefer:
node-version: ["20.18.3"]
Using pinned versions ensures consistent builds across different environments and times, prevents unexpected breaking changes, and makes security audits more reliable.
Avoid implementing logging calls that could trigger recursive logging patterns, which can lead to infinite loops and program crashes. This is particularly important in logging handlers, exporters, and instrumentation code.
Key guidelines:
Example of safe logging implementation:
# Do not add any logging.log statements to this function
# they can trigger recursive calls that crash the program
def emit(self, log_data):
try:
self._exporter.export((log_data,))
except Exception:
warnings.warn(f"Exception while exporting logs: {traceback.format_exc()}")
For logging handlers and processors, consider using suppression flags:
from opentelemetry.context import attach, set_value
def emit(self):
token = attach(set_value(_SUPPRESS_INSTRUMENTATION_KEY, True))
try:
# Logging code here
pass
finally:
detach(token)
Comments should explain “why” and “how” rather than restating what is already obvious from the code. Focus on providing additional context, usage examples, or explanations that would help other developers understand the intent and proper usage of the code.
Good practice:
Example - Instead of:
# Singleton of meter.get_label_set() with zero arguments
Write:
# This singleton instance is used as the default when no labels are provided,
# avoiding unnecessary object creation during high-frequency metric recording
Example - For complex attribute filtering:
# Filter out attributes that are already included in point_attributes to avoid
# duplication, keeping only unique attributes from the original measurement
current_attributes = (
{
k: v
for k, v in self.__attributes.items()
if k not in point_attributes
}
if self.__attributes
else None
)
When documenting APIs, be specific about parameter purposes - for example, instead of simply listing “port” and “address” parameters, clarify that they are “the port and address where the service listens for requests from Prometheus.”
Avoid using GitHub workflow template expressions ${{ }} directly in shell commands within run: steps, as this can lead to template injection vulnerabilities. Untrusted input like branch names, PR titles, or commit messages can be crafted to execute arbitrary code since GitHub template expansion happens before workflow execution.
Instead, pass GitHub context values through environment variables and use shell variable expansion ${...} to access them safely. This ensures the values are treated as literal strings rather than executable code.
Example of vulnerable code:
- run: ./script.sh "$(./get_version.sh ${{ github.ref_name }})"
Secure alternative:
- run: ./script.sh "$(./get_version.sh ${GH_REF_NAME})"
env:
GH_REF_NAME: ${{ github.ref_name }}
This pattern is especially critical for workflows that can be triggered by external contributors or when processing any user-controllable input from GitHub context variables.
When working with configuration files and values, prefer dedicated tools and canonical sources over manual text manipulation or secondary configuration files. Use language-specific tools for modifications (e.g., go mod edit -go=1.23 instead of sed commands on go.mod) and extract configuration values from their primary sources rather than derived files.
For example, instead of:
# Avoid manual text manipulation
sed -i.bak -E "s/^go [0-9]+\.[0-9]+\.[0-9]+$/go ${NEW_VERSION}.0/" go.mod
# Avoid secondary sources
REQUIRED_GO_VERSION=$(grep 'version:' .promu.yml | awk '{print $2}')
Prefer:
# Use proper tooling
go mod edit -go=${NEW_VERSION}.0
# Use canonical sources
MIN_GO_VERSION=$(awk '/^go / {print $2}' go.mod)
This approach reduces fragility, improves maintainability, and ensures configuration changes are handled through proper validation and formatting provided by the dedicated tools.
Inline objects and functions created within render methods are recreated on every render, causing unnecessary re-renders of child components when passed as props. This impacts performance by triggering component updates even when the actual functionality hasn’t changed.
To optimize performance, extract inline objects and functions using memoization techniques:
useCallback for inline functionsuseMemo for inline objectsExample of the problem:
// Problematic - creates new object on every render
<Table
locale={{
emptyText: 'No data available'
}}
/>
// Optimized - memoize the object
const tableLocale = useMemo(() => ({
emptyText: 'No data available'
}), []);
<Table locale={tableLocale} />
This practice prevents unnecessary child component re-renders and improves overall application performance.
Always update dependencies with known security vulnerabilities to their patched versions. Dependencies with security issues can introduce vulnerabilities into your application even if your own code is secure.
When you discover a dependency with a security vulnerability:
Consider implementing automated dependency scanning in your CI/CD pipeline to proactively identify vulnerabilities before they reach production.
Example:
// Before: Vulnerable dependency
require (
golang.org/x/net v0.36.0 // Has CVE-2025-22872 vulnerability
)
// After: Updated to patched version
require (
golang.org/x/net v0.38.0 // Fixes CVE-2025-22872
)
Write tests that closely mirror real user interactions and application behavior rather than relying on artificial test-specific constructs. This approach creates more robust and maintainable tests.
Key practices:
Use semantic queries over test IDs: Prefer getByRole, getByLabelText, getByText instead of data-testid attributes. These queries align with how users actually interact with the UI and make tests more resilient to implementation changes.
Use MSW for API testing: When testing components that fetch data via hooks, use Mock Service Worker (MSW) instead of directly mocking the hooks. This ensures the complete render cycle is tested and maintains realistic data flow.
Example:
// ❌ Avoid: Artificial test constructs
<div data-testid="funnel-graph-legend-column">
{/* content */}
</div>
// Test
expect(getByTestId('funnel-graph-legend-column')).toBeInTheDocument();
// ❌ Avoid: Direct hook mocking
jest.mock('hooks/queryBuilder/useGetAggregateKeys', () => ({
useGetAggregateKeys: jest.fn().mockReturnValue(mockData)
}));
// ✅ Prefer: Semantic queries
<div role="region" aria-label="Funnel graph legend">
{/* content */}
</div>
// Test
expect(getByRole('region', { name: 'Funnel graph legend' })).toBeInTheDocument();
// ✅ Prefer: MSW for API mocking
// Set up MSW handlers to intercept actual API calls
This approach reduces production bundle size by avoiding test-specific attributes and creates tests that break when user experience is actually impacted, not just when implementation details change.
Create deterministic and reliable test cases by following these guidelines:
boundaries = [randint(0, 1000)]
boundaries = [500] # Fixed, predictable value
2. For timing-sensitive tests:
- Mark tests as flaky if they depend on timing:
```python
@mark.flaky(reruns=3, reruns_delay=1)
def test_timing_dependent():
# Test implementation
self.assertTrue((after_export - before_export) < 1e9)
self.assertLess(after_export - before_export, 1e9)
self.assertAlmostEqual(after_export - before_export, expected, delta=1e9)
3. Skip platform-specific tests rather than adding workarounds:
```python
@mark.skipif(
system() == "Windows",
reason="Tests fail due to Windows time_ns resolution limitations"
)
def test_platform_specific():
# Test implementation
These practices ensure tests are reproducible, maintainable, and provide clear failure messages when issues occur.
Use standard capitalization for acronyms in all identifiers, method names, variables, and other code elements. Common acronyms like ‘API’, ‘URL’, ‘JSON’, ‘HTML’, and ‘XML’ should be written in all uppercase letters.
This applies to all contexts:
getAPIKeys instead of getApiKeysuserAPIConfig instead of userApiConfig"name": "Get API Keys" instead of "name": "Get Api Keys"For example, in our API endpoint definitions:
{
"_type": "endpoint",
"name": "Get API Keys", // Correct - acronym fully capitalized
...
}
Instead of:
{
"_type": "endpoint",
"name": "Get Api Keys", // Incorrect - inconsistent acronym capitalization
...
}
Consistent acronym capitalization improves code readability, maintains professional standards across the codebase, and helps ensure interface consistency for users.
Replace non-deterministic timing patterns with reliable alternatives to prevent flaky tests. This includes:
// Good await page.waitForSelector(‘[data-testid=”element”]’, { state: ‘visible’, timeout: 5000 });
2. Use fixed timestamps for time-dependent tests:
```typescript
// Bad
const cutoffDate = new Date(Date.now() + 24*60*60*1000);
// Good
const cutoffDate = new Date('2024-01-01T00:00:00Z');
// Good const fixedDate = new Date(‘2024-01-01T00:00:00Z’); jest.useFakeTimers().setSystemTime(fixedDate); const job = await createJob({ timestamp: new Date() });
This approach reduces test flakiness, improves CI reliability, and makes tests more maintainable by removing timing-dependent assumptions.
---
## Materialize database indexes
<!-- source: langfuse/langfuse | topic: Database | language: Sql | updated: 2025-04-09 -->
Always materialize database indexes after creation to ensure they are fully built and immediately available for queries. This is especially important in migration scripts where database performance shouldn't be compromised during or after the migration process. Without materialization, indexes may not be immediately usable, causing potential performance issues until the background materialization completes.
Example in ClickHouse:
```sql
-- Always follow index creation with materialization
ALTER TABLE scores ON CLUSTER default ADD INDEX IF NOT EXISTS idx_project_session (project_id, session_id) TYPE bloom_filter(0.001) GRANULARITY 1;
ALTER TABLE scores ON CLUSTER default MATERIALIZE INDEX IF EXISTS idx_project_session;
This two-step approach ensures that indexes are not only created but also immediately available for use in subsequent queries.
Place configuration files and settings at the appropriate level in your repository structure. In monorepos, some configurations must be at the root level while others belong at the package level.
Key guidelines:
.gitignore to prevent accidental commitsExample from a monorepo structure:
// Root package.json - for patches and shared overrides
{
"resolutions": {
"overrides": {
"jsonpath-plus": "10.0.7",
"katex": "^0.16.21",
"openid-client": "5.6.5"
},
"patchedDependencies": {
"next-auth@4.24.11": "patches/next-auth@4.24.11.patch",
"openid-client@5.6.5": "patches/openid-client@5.6.5.patch"
}
}
}
// .gitignore - exclude ephemeral files
web/test-results/.last-run.json
Inappropriate placement of configuration can cause functionality issues or repository pollution.
Never include plaintext passwords or credentials in connection strings, configuration files, or documentation examples. This creates security vulnerabilities as credentials could be exposed in logs, version control, or to unauthorized personnel.
Instead:
Example - Replace this insecure approach:
postgresql://user:password@host:5432/dbname
With a more secure alternative:
postgresql://${DB_USER}:${DB_PASSWORD}@host:5432/dbname
Where environment variables or a secrets manager handles the actual credentials. When providing documentation or examples, always include warnings about security implications of different connection methods.
Sensitive credentials such as API keys, passwords, access tokens, and secret keys should never be stored in plain text in databases or configuration files. This creates significant security risks if the database is compromised or if unauthorized access occurs.
Instead:
Example of an improved approach:
-- CreateTable
CREATE TABLE "blob_storage_integrations" (
"project_id" TEXT NOT NULL,
"bucket_name" TEXT NOT NULL,
"prefix" TEXT NOT NULL,
"access_key_id" TEXT NOT NULL,
"encrypted_secret_key" TEXT NOT NULL,
"encryption_iv" TEXT NOT NULL,
In your application code, implement proper encryption/decryption methods to handle these sensitive values when needed.
When implementing error handling logic, be deliberate about which exception types you catch and where you handle them.
Exception, not BaseException - Classes that directly inherit from BaseException (like GeneratorExit or KeyboardInterrupt) are not technical errors and should generally propagate:# Good
try:
# operation that may fail
except Exception as exc: # Only catches errors, not control flow exceptions
record_error(exc)
# Bad
try:
# operation that may fail
except BaseException as exc: # Catches everything including KeyboardInterrupt
record_error(exc)
# Good: Handle file access errors in file-reading functions
def _read_file(file_path: str) -> Optional[bytes]:
try:
with open(file_path, "rb") as file:
return file.read()
except FileNotFoundError as e:
logger.exception(f"Failed to read file: {e.filename}")
return None
# Bad: Force higher-level code to handle file-specific errors
def _read_file(file_path: str) -> bytes:
with open(file_path, "rb") as file:
return file.read()
# FileNotFoundError bubbles up to caller
Preserve valuable error information - When handling exceptions, ensure meaningful error details are preserved in logs or return values.
Balance fail-fast and graceful degradation - Consider the context:
Build processes should provide clear, visible feedback about the operations being performed rather than suppressing output or running silently. This transparency helps developers understand what’s happening during builds, aids in debugging when issues occur, and builds confidence that expected operations (like code signing) are actually executing.
Avoid suppressing command echoing unnecessarily and add informative echo statements for significant operations that might otherwise be invisible to users.
Example:
# Good - visible commands and informative messages
common-build: promu
@echo ">> building binaries"
$(PROMU) build --prefix $(PREFIX) $(PROMU_BINARIES)
@if [ "$(GOHOSTOS)" = "darwin" ] && command -v codesign > /dev/null 2>&1; then \
echo ">> signing binaries for macOS"; \
codesign --sign - --force --preserve-metadata=entitlements,requirements,flags,runtime $(PREFIX); \
fi
# Avoid - suppressed output leaves users uncertain
@$(PROMU) build --prefix $(PREFIX) $(PROMU_BINARIES) && \
codesign --sign - --force $(PREFIX) >/dev/null 2>&1
This practice is especially important in CI/CD environments where build logs are the primary way to understand and debug automated processes.
Ensure configuration files use tool-specific syntax and conventions while leveraging appropriate defaults. Be intentional about version constraints and conditionals in dependency specifications.
For tool configurations:
pyright: ignore [error_code] instead of type: ignore [error_code]).gitignore files)For dependency specifications:
Example:
# Good: Using tool-specific configuration with appropriate defaults
[tool.ruff]
target-version = "py38"
line-length = 79
# Not duplicating default excludes that already cover .tox, .venv, etc.
# Good: Using specific type ignore syntax
# In Python file: # pyright: ignore [specificError]
# For whole file: # pyright: strict, reportUnusedVariable=none
# Good: Proper dependency versioning with conditionals
dependencies = [
"Deprecated >= 1.2.6",
"importlib-metadata >= 6.0, <= 8.2.0; python_version < '3.10'",
]
Ensure naming conventions are consistent across your codebase and related repositories. When naming commands, functions, variables, or attributes:
PROCESS_COMMAND_ARGS vs. attribute names like process.command_args)Example:
# Inconsistent naming (mixing conventions)
tox -e pyright # in this repo
tox -e typecheck # in related repo
# Consistent naming (following established conventions)
tox -e typecheck # in both repos
This guideline improves code readability, reduces cognitive load when working across multiple repositories, and helps ensure documentation accurately represents code elements.
Structure configuration files like tox.ini to maximize maintainability and efficiency. Use factor prefixes (e.g., “test-opentelemetry:”, “lint:”) to install packages only for specific environments, avoiding unnecessary dependencies. When naming environments, avoid characters that have special meaning in the tool (e.g., use “precommit” instead of “pre-commit” since tox treats hyphens as factor separators).
Example:
[testenv]
deps =
-c dev-requirements.txt
# Install specific packages only for lint environments
lint: -r lint-requirements.txt
# Install specific packages only for test environments
test-opentelemetry: pytest
test-opentelemetry: pytest-benchmark
This approach reduces test environment setup time and makes dependencies explicit for each environment type. Consider using tools like pip-sync to keep environments in sync with requirements.txt changes. For complex projects, evaluate whether specialized requirements files for different virtual environments improve clarity or add unnecessary complexity.
Maintain consistent and informative changelog documentation by following these practices:
Example:
# Changelog
## Unreleased
- [BREAKING] Remove `opentelemetry.semconv.attributes.network_attributes.NETWORK_INTERFACE_NAME` due to deprecation in the specification
- Add `Final` decorator to constants to prevent accidental reassignment in strongly-typed code
## v1.0.0 (2023-04-01)
- Initial stable release
- Fix span context manager typing by using ParamSpec from typing_extensions
This approach ensures users and developers can easily track changes, understand their impact, and prepare for upgrades appropriately.
Place code in the most specific and appropriate location to improve findability and maintainability. Follow these principles:
Example - Instead of:
# generic util.py
def sanitize(key): ...
class CustomCollector:
def _translate_to_prometheus(self, metric_record):
# using global sanitize function
label_keys.append(sanitize(label_key))
Better approach:
# prometheus_translator.py
class CustomCollector:
def _translate_to_prometheus(self, metric_record):
def sanitize(key): # Function in most specific scope
# sanitization logic
pass
label_keys.append(sanitize(label_key))
This organization improves code findability, reduces coupling, and makes the codebase more maintainable.
When building binaries for macOS (Darwin), always force re-sign them to prevent compatibility issues caused by pre-existing signatures. Use the --force flag with codesign to overwrite any existing signatures that might interfere with binary execution.
Code signing is a critical security mechanism that ensures binary integrity and authenticity. However, pre-existing signatures from previous builds or different environments can cause runtime compatibility issues on macOS systems. By force re-signing binaries, you ensure a clean, consistent signature that won’t conflict with the target execution environment.
Example implementation:
common-build: promu
@echo ">> building binaries"
@$(PROMU) build --prefix $(PREFIX) $(PROMU_BINARIES) && \
if [ "$(GOHOSTOS)" = "darwin" ] && command -v codesign > /dev/null 2>&1; then \
codesign --sign - --force --preserve-metadata=entitlements,requirements,flags,runtime $(PREFIX) >/dev/null 2>&1; \
fi
This practice ensures that macOS binaries have consistent, valid signatures while maintaining security properties through proper code signing.
Always use explicit, specific configurations in CI/CD pipelines to prevent ambiguity and conflicts. This includes:
Clear environment naming - Use specific environment names to avoid test execution conflicts between components. For example, prefer py3{8,9,10,11,12}-test-opentelemetry-proto-protobuf5 over more ambiguous patterns like py3{8,9,10,11,12}-test-opentelemetry-proto-{0,1}.
Full paths for output files - Always specify complete paths for output artifacts to ensure they’re generated in the expected location: ```
pytest {toxinidir}/opentelemetry-sdk/benchmarks –benchmark-json={toxinidir}/opentelemetry-sdk/sdk-benchmark.json {posargs}
pytest {toxinidir}/opentelemetry-sdk/benchmarks {posargs} –benchmark-json=sdk-benchmark.json
3. **Explicit dependency references** - When dependencies must be pinned to specific versions or commits, clearly document why with comments and references to pending PRs:
CONTRIB_REPO_SHA=e36889568d1c57c5f6e1dfa65a73519a8eb6607d
These practices improve pipeline reliability, reduce debugging time, and make CI configurations more maintainable across the team.
---
## Adapt for linter compatibility
<!-- source: open-telemetry/opentelemetry-python | topic: Code Style | language: Other | updated: 2024-10-02 -->
When writing or modifying code, design patterns and templates to be compatible with linting tools. This is especially important for generated code and template systems.
For templates that generate code, consider how linters will interpret the output:
```python
# Before - may trigger lint warnings when a variable has an inline comment
# VARIABLE: Final = 42
"""
Deprecated: This is being phased out.
"""
# After - linter-friendly approach
# VARIABLE: Final = 42
# Deprecated: This is being phased out.
For generated code (like protobuf files), prioritize proper lint fixes rather than disabling linters. When lint issues are complex, consider addressing them in dedicated PRs to maintain code quality without blocking other changes. Ensure that tooling configurations (like those in tox.ini) properly handle linting requirements for all code types in your project.
Select data structures based on your specific access patterns and performance requirements. Using the right data structure can simplify your code and improve performance significantly.
For queue-like operations where you need to add/remove from both ends efficiently, use specialized collections like deque instead of plain lists:
# Instead of this:
self._metrics_to_export = []
# ...later...
self._metrics_to_export.append(metric_records)
# ...and in another method...
self._metrics_to_export.remove(metric_batch) # Inefficient for large lists
# Use this:
from collections import deque
self._metrics_to_export = deque()
# ...later...
self._metrics_to_export.append(metric_records)
# ...and in another method...
metric_batch = self._metrics_to_export.popleft() # O(1) operation
When working with collections:
For example, when iterating and modifying:
# Instead of this:
for metric_batch in self._metrics_to_export:
# process metric_batch
self._metrics_to_export.remove(metric_batch) # DANGER! Modifying during iteration
# Use this:
while self._metrics_to_export:
metric_batch = self._metrics_to_export.popleft()
# process metric_batch
Using npx --yes bypasses security prompts and automatically installs packages without verification, which could lead to supply chain attacks if package names are typosquatted or compromised. Always install required tools as explicit dependencies in your project.
Instead of this (risky):
npx --yes @datadog/datadog-ci sourcemaps upload "$DIST_PATH"
Do this instead (safer):
# In package.json, add as a dev dependency:
# "@datadog/datadog-ci": "^x.y.z"
# Then in your script:
npx @datadog/datadog-ci sourcemaps upload "$DIST_PATH"
By explicitly declaring dependencies, you ensure consistent versions, improve security posture, and enable your team to review all dependencies during security audits.
Return empty collections (lists, dicts, sets) instead of None when representing empty states. This reduces null checking boilerplate, prevents null reference errors, and makes code more predictable. When a function returns a collection type, an empty collection communicates “no items” more clearly than None and allows direct operations without defensive checks.
Example:
# Not recommended
def extract_tags(span):
if not span.tags:
return None # Forces callers to check for None
return [tag for tag in span.tags]
# Recommended
def extract_tags(span):
if not span.tags:
return [] # Callers can immediately iterate/operate on result
return [tag for tag in span.tags]
# Usage is simpler and safer
tags = extract_tags(span)
for tag in tags: # Works directly, no None check needed
process_tag(tag)
This pattern:
For error conditions or invalid states, raise exceptions or use Optional types instead of returning None.
Always use exact version pinning (==) for dependencies in requirements files rather than version ranges. This ensures build reproducibility and prevents unexpected behavior when dependencies release new versions.
For dependencies that must work across multiple Python versions, identify a compatible version that works for all supported Python versions rather than using version ranges. This approach prevents compatibility issues like the one seen with pre-commit, where newer versions dropped support for Python 3.8.
If conditional dependencies are needed, still use fixed versions in the conditionals.
# Good
importlib-metadata==6.11.0
pre-commit==3.5.0 # Compatible with all supported Python versions
# Avoid
importlib-metadata<9.0.0
importlib-metadata<=8.2.0
Additionally, maintain consistency by using the same version of a dependency throughout the entire repository. This prevents subtle bugs that can occur when different components use different versions of the same library.
This approach ensures consistent behavior across all development and CI environments, improving reproducibility and reducing debugging time.
When specifying observability frameworks like OpenTelemetry in requirements files, use the compatible release operator (~=) with only the major and minor version numbers, omitting the patch version. This ensures you receive bug fixes in patch releases while protecting against potential breaking changes in minor versions.
Example:
# Recommended
opentelemetry-api~=1.25
# Not recommended
opentelemetry-api>=1.25.0
opentelemetry-api~=1.25.0
This approach balances stability with bug fixes for your observability stack, ensuring consistent telemetry collection across environments.
Use consistent Python naming conventions to improve code readability and maintainability:
def setifnotnone(dic, key, value):
def set_if_not_none(dic, key, value):
2. Choose descriptive, semantic names that reflect purpose:
```python
# Bad
lst = [] # unclear what this list tracks
# Good
calls = [] # clearly indicates list tracks function calls
self.currentValue = None self.previous_cumulative_value = None
self._value = None self._previous_value = None
---
## Include practical examples
<!-- source: getsentry/sentry-php | topic: Documentation | language: Markdown | updated: 2024-06-04 -->
Enhance technical documentation with clear, practical code examples that demonstrate usage. This applies to:
1. Changelog entries for new features
2. API documentation
3. Contributing guidelines
4. Migration/upgrade guides
Good documentation with examples reduces confusion, speeds up implementation, and prevents common mistakes. When documenting changes or new functionality, clearly explain what it does and provide sample code showing proper usage.
Example of an improved changelog entry:
Add timing span when emiting a timing metric (#1717)
// Example usage
$metric = new Timing('database.query', 42.0);
$metric->setSpanStatus(SpanStatus::OK);
$metric->setUnit(DurationUnit::MILLISECOND);
```
For migration guides, show both old and new approaches side by side to help users understand the transition. Clear examples are especially important when explaining complex changes or features that might be unintuitive.
When exporting observability data (metrics, traces, logs) to external systems, properly sanitize all data fields to ensure compatibility and consistency. Key requirements:
Example:
def sanitize_metric_name(name: str) -> str:
# Handle leading digit
if name and name[0].isdigit():
name = "_" + name
# Replace invalid chars with underscore
return re.sub(r"[^a-zA-Z0-9_]", "_", name)
def sanitize_label_key(key: str) -> str:
# Remove special chars
return re.sub(r"[^a-zA-Z0-9]", "_", key)
def format_label_value(value: Any) -> str:
# Convert to string and escape
return str(value).replace("\\", "\\\\").replace("\n", "\\n")
# Usage
metric_name = sanitize_metric_name("2xx.success.rate") # "_2xx_success_rate"
label_key = sanitize_label_key("status.code") # "status_code"
label_value = format_label_value(200) # "200"
This ensures reliable data export and prevents issues with downstream observability systems while maintaining semantic meaning of the original data.
Implement caching mechanisms as opt-in features with explicit configuration options rather than as defaults. This approach prevents unexpected behavior and allows teams to deliberately enable caching where beneficial.
Key implementation guidelines:
Example implementation:
export function runRequest(
datasourceRequest: DataQueryRequest,
options: RunRequestOptions = {}
): Observable<PanelData> {
// Cache configuration with defaults
const cacheConfig = {
enabled: options.enableCache ?? false,
cacheErrorResponses: false,
// other cache options
};
// Generate cache key only if caching is enabled
const cacheKey = cacheConfig.enabled ? generateCacheKey(datasourceRequest) : null;
// Check cache first if enabled
if (cacheConfig.enabled && cacheKey && cache.has(cacheKey)) {
return of(cache.get(cacheKey));
}
// Proceed with request
// ...
// Only cache successful responses
if (cacheConfig.enabled && cacheKey && !state.panelData.error) {
cache.set(cacheKey, state.panelData);
}
}
This approach allows for shared caching across different parts of the application while avoiding the pitfall of multiple layers of cache that could lead to cache invalidation challenges.
When implementing design tokens in code, ensure values accurately represent the intended visual outcome, even if design tools have limitations. Use proper unit types that correctly translate to UI rendering rather than arbitrary numeric values.
For example, when defining border-radius tokens for circular elements, use:
"circle": {
"value": "100%",
"type": "borderRadius",
"description": "Used to generate full circle elements"
}
Instead of using large numeric values like 100000 which may not scale properly with element dimensions.
While maintaining comprehensive token scales for future flexibility is valuable, clearly document which tokens are intended for direct use versus internal reference. Add descriptive comments that explain the purpose and usage context of each token group. This improves code readability and prevents misapplication of foundation-level tokens that should only be referenced by semantic tokens.
Never use panic() for error handling in production code paths, especially in core engine components. Instead, handle errors gracefully by returning appropriate values or ignoring problematic data with optional warnings.
When encountering errors during operations like histogram arithmetic or data processing, prefer these approaches:
Example of problematic code:
res, err := hlhs.Copy().Add(hrhs)
if err != nil {
panic(err) // DON'T DO THIS
}
Better approach:
res, err := hlhs.Copy().Add(hrhs)
if err != nil {
return 0, nil, false // Gracefully indicate failure
}
This ensures system stability and prevents cascading failures from individual data processing errors. Panics should only be reserved for truly unrecoverable programming errors during development, not for handling expected error conditions in production.
When implementing role-based access control (RBAC), ensure you use the correct permission verification method based on the security context. Distinguish between:
User-level permissions: Use hasPermission() when checking if the current user has a specific permission, regardless of the object being accessed.
Object-specific permissions: Use hasPermissionInMetadata() when verifying if access is allowed for a specific object based on its metadata.
Using the wrong permission check can lead to security vulnerabilities or overly restrictive access. Always confirm which level of permission verification is required for your use case.
Example:
// For user-level permission check
if (!contextSrv.hasPermission(AccessControlAction.PluginsWrite)) {
// Handle unauthorized access
}
// For object-specific permission check
if (!contextSrv.hasPermissionInMetadata(AccessControlAction.PluginsWrite, plugin)) {
// Handle unauthorized access
}
Always propagate errors appropriately by rethrowing caught exceptions and maintaining error context. Catch exceptions only when you can handle them meaningfully, and ensure errors aren’t silently swallowed.
Key principles:
Example:
// Bad - Swallowing the error
try {
$result = $callback();
} catch (\Throwable $e) {
$status = CheckInStatus::error();
}
// Good - Proper error propagation
try {
$result = $callback();
} catch (\Throwable $e) {
$status = CheckInStatus::error();
throw $e; // Rethrow to maintain error context
}
// Good - Focused exception handling
try {
$promise->wait();
} catch (\Throwable $e) {
return null; // Explicit handling with clear intent
}
Organize your test suite by using data providers to consolidate similar test cases rather than creating multiple test methods with duplicated logic. This improves maintainability, increases test coverage, and makes adding new test scenarios easier.
When you have multiple test cases that follow the same testing pattern but with different input/output values, refactor them using a data provider:
/**
* @dataProvider messageDataProvider
*/
public function testMessage(string $message, array $params, ?string $formatted, array $expected): void
{
$event = new Event();
$event->setMessage($message, $params, $formatted);
$data = $event->toArray();
$this->assertArrayHasKey('message', $data);
$this->assertSame($expected, $data['message']);
}
public function messageDataProvider(): \Generator
{
// Test case 1: Basic message
yield [
'foo',
[],
null,
['message' => 'foo'],
];
// Test case 2: Message with parameters and formatted value
yield [
'foo @bar',
['@bar' => 'bar'],
'foo bar',
[
'message' => 'foo @bar',
'params' => ['@bar' => 'bar'],
'formatted' => 'foo bar',
],
];
}
Data providers also help document the relationship between inputs and expected outputs, making your tests serve as better documentation. Remember to give your data provider methods descriptive names and consider adding comments for complex test cases.
Configure CI workflows to optimize both comprehensive testing and resource usage. For resource-intensive tests that may cause issues (like OOM errors with code coverage), split test suites and run coverage selectively:
# Example: Split test suites for better resource management
- name: Run unit tests with coverage
run: vendor/bin/phpunit --testsuite unit --coverage-clover=coverage.xml
- name: Run resource-intensive tests without coverage
run: vendor/bin/phpunit --testsuite intensive --no-coverage
Include quick tests (like benchmarks under a few seconds) in regular CI runs to prevent code rot, but consider separate workflows for longer-running tests. Use CI platform features like skip_branch_with_pr: true to prevent duplicate builds when PRs are open, optimizing overall CI resource usage.
When specifying dependency version constraints, always include lower version bounds that exclude versions with known security vulnerabilities. For dependencies used directly in your code (not just transient dependencies), you have a responsibility to prevent users from inadvertently installing vulnerable versions, as this makes your own package vulnerable.
Example:
"require": {
"guzzlehttp/psr7": "^1.8.4|^2.1.1", // Good: specifies minimum versions without vulnerabilities
// "guzzlehttp/psr7": "^1.0|^2.0", // Bad: allows versions with known CVEs
}
This practice helps protect your users from security issues like CVE-2022-24775 and similar vulnerabilities. Remember that even if users are responsible for their overall dependency management, you are responsible for the security implications of the code you distribute.
Always be explicit and consistent when handling null values to improve code clarity and prevent subtle bugs. This applies to null checks, nullable type declarations, and function parameters.
For null checks:
null !== $variable or $variable === null instead of just if ($variable)For type declarations:
?) for parameters and return types that can be nullFor function parameters:
// Bad
public function doSomething($client)
{
if ($client) {
// ...
}
}
// Good
public function doSomething(?ClientInterface $client = null): ?string
{
if (null !== $client) {
// ...
}
}
This pattern makes intent clear, improves static analysis capabilities, and provides better autocomplete support in IDEs. It also prevents confusion between false, empty strings, zero, and null values in conditionals.
When handling files in your application, minimize memory usage to optimize performance. Follow these guidelines:
Avoid multiple reads: Read file content only once and reuse it throughout your code. Each read operation loads the entire file into memory.
Use metadata functions: When you only need file information (not content), use metadata functions like filesize() instead of reading the file.
Consider streaming: For large files, use stream operations instead of loading the entire content into memory.
Example refactoring:
// Before: Inefficient - reads file twice and holds it twice in memory
public static function fromFile(string $filename, string $contentType): self
{
$data = file_get_contents($filename); // First read
$attachmentItemHeader = [
'type' => 'attachment',
'filename' => $filename,
'content_type' => $contentType,
'length' => strlen($data), // Requires second read or keeping in memory
];
return new self($data, $filename, $contentType);
}
// After: Efficient - reads file once, uses filesize() for metadata
public static function fromFile(string $filename, string $contentType): self
{
// Use metadata function instead of reading content
$fileLength = filesize($filename);
$attachmentItemHeader = [
'type' => 'attachment',
'filename' => $filename,
'content_type' => $contentType,
'length' => $fileLength, // No need to read content for length
];
// Read content only when needed
$data = file_get_contents($filename);
return new self($data, $filename, $contentType);
}
For very large files, consider using stream operations to process the file incrementally rather than loading it entirely into memory.
Break long lines and complex structures across multiple lines to improve code readability. Follow these guidelines:
Example of proper formatting:
/**
* This is a long docblock description that would exceed
* the 80 character limit if written on a single line.
*
* @param string $param Description of the parameter
*/
public function example($param)
{
$data = [
'first_key' => 'value1',
'second_key' => 'value2',
'third_key' => 'value3',
];
$object->methodOne()
->methodTwo()
->methodThree();
}
Instead of:
/** This is a long docblock description that would exceed the 80 character limit if written on a single line. */
public function example($param)
{
$data = ['first_key' => 'value1', 'second_key' => 'value2', 'third_key' => 'value3'];
$object->methodOne()->methodTwo()->methodThree();
}
Choose meaningful, self-explanatory names for variables, parameters, properties, and methods that clearly convey their purpose. Avoid abbreviations and acronyms unless they’re widely understood industry standards.
For boolean variables, use prefixes like is, has, or should to improve readability:
// Better
private $isFrozen = false;
// Instead of
private $frozen = false;
Use full descriptive names rather than acronyms or abbreviations for domain-specific concepts:
// Better - reduces cognitive load for readers
$samplingContext = DynamicSamplingContext::fromTransaction($this->transaction);
// Instead of
$dsc = DynamicSamplingContext::fromTransaction($this->transaction);
Maintain consistency in parameter naming across related methods and classes:
// Better - consistent naming pattern
public function fromArray(array $data): self
public function sanitizeData(array $data): array
// Instead of mixing naming styles
public function fromArray(array $array): self
public function sanitizeTags(array $tags): array
When naming test data variables, choose names that clearly describe what they represent rather than their type:
// Better
public function testToArrayWithMessage(array $messageArguments, $expectedValue)
// Instead of
public function testToArrayWithMessage(array $message, $expectedValue)
When specifying dependency version constraints in configuration files, be deliberate about minimum versions based on required features and compatibility requirements. Use the lowest compatible version constraint to maximize project compatibility, but ensure specific feature requirements are met.
For example, if your code requires a specific feature added in version 1.6 of a package:
// Good - Specific minimum version based on feature requirements
"monolog/monolog": "^1.6|^2.0|^3.0",
// Bad - Unnecessarily restrictive
"monolog/monolog": "^1.3|^2.0",
When upgrading dependency versions, consider broader compatibility impacts with other packages in your ecosystem. Use version ranges (e.g., ^2.19|^3.4) when necessary to maintain compatibility with different environments. Document the reasoning for specific version constraints when they’re not obvious, especially when choosing a particular minimum version for feature availability.
When using regular expressions, optimize for both performance and precision to ensure efficient and accurate pattern matching:
// Less efficient
preg_replace('/0x[a-fA-F0-9]+$/', '', $string);
// More efficient
preg_replace('/(?:0x)[a-fA-F0-9]+$/', '', $string);
// Too permissive for hex format
preg_replace('/0x[a-zA-Z0-9]+$/', '', $string);
// Correct for hex format
preg_replace('/0x[a-fA-F0-9]+$/', '', $string);
// Without anchors - could match substrings
expect($error)->toMatch('/The option "foo" does not exist./');
// With anchors and escaped special characters - matches whole string precisely
expect($error)->toMatch('/^The option "foo" does not exist\.$/')
These optimizations lead to more predictable pattern matching, fewer bugs in data processing, and better performance when dealing with large datasets.
When adding or modifying configuration options, ensure they are documented with complete context. Group related changes together (like deprecation and default value changes), include reference numbers, and explain the purpose and effect of configuration options. This helps users understand how to properly configure the system without confusion.
For example, instead of:
- Add `in_app_include` option
Use:
- Add `in_app_include` option to whitelist paths that should be marked as part of the app (#909)
Similarly, related configuration changes should be documented together:
- Lower the default `send_attempts` option to `0` (disabling retries) and deprecate this option in favor of the new retry mechanism (#1312)
When documenting API changes, particularly breaking changes, follow these practices to ensure clarity and ease of migration:
- The following methods now return `EventId` instead of `string`:
- `ClientInterface::captureMessage()`
- `ClientInterface::captureException()`
- `HubInterface::captureEvent()`
- Removed the following methods from `ClientBuilderInterface`, use `ClientBuilderInterface::setTransportFactory()` instead:
- `setUriFactory()`
- `setHttpClient()`
- Refactor to support the [Unified API SDK specs](https://docs.sentry.io/development/sdk-dev/unified-api/)
- Make the `StacktraceBuilder` class part of the public API and add the `Client::getStacktraceBuilder()` method
These practices ensure that developers can easily understand changes, assess impact, and migrate their code efficiently.
Design error handling systems that respect runtime changes to error settings rather than fixing configurations at initialization time. When handling errors, dynamically evaluate error reporting flags to allow for temporary error silencing and context-specific error handling behavior. Preserve user-specified error levels when processing exceptions to ensure intended error reporting behavior is maintained throughout the error handling pipeline.
// Good: Dynamically evaluate error settings during error handling
function errorHandler($errno, $errstr, $errfile, $errline) {
// Check current error reporting settings at runtime
if (!(error_reporting() & $errno)) {
// Error is silenced with @ operator or error_reporting settings
return;
}
// Handle error based on current settings
// ...
}
// Bad: Fixed error configuration at initialization
$errorTypes = error_reporting(); // Captured once at setup
function badErrorHandler($errno, $errstr, $errfile, $errline) {
global $errorTypes;
if (!($errorTypes & $errno)) { // Uses fixed setting from initialization
return;
}
// ...
}
Support multiple configuration formats where appropriate (string, callable, object) to accommodate different frameworks and usage patterns. This improves developer experience across different ecosystems (like Laravel, Symfony) while maintaining type safety through proper validation.
For example, when implementing a configuration option that accepts a callback:
/**
* Gets a callback that will be invoked when we sample a Transaction.
*
* @psalm-return \Sentry\Tracing\TracesSamplerInterface|callable(\Sentry\Tracing\SamplingContext): float
*/
public function getTracesSampler()
{
if (\is_string($this->options['traces_sampler']) && class_exists($this->options['traces_sampler'])) {
return new $this->options['traces_sampler']();
}
return $this->options['traces_sampler'];
}
Ensure proper validation:
// In configureOptions method:
$resolver->setAllowedTypes('traces_sampler', ['null', 'string', 'callable', 'object']);
For values that should be configurable across projects, extract them as options or constants rather than hardcoding them:
// Instead of hardcoding:
$keysToRemove = ['authorization', 'cookie', 'set-cookie', 'remote_addr'];
// Extract to a constant:
private const HEADERS_TO_SANITIZE = ['authorization', 'cookie', 'set-cookie', 'remote_addr'];
// Or make it configurable through integration options
This approach allows users to configure your library in ways that match their framework conventions while maintaining strong typing and validation.
Documentation should convey purpose and behavior, not just replicate code structure. Each docblock should explain what a component does and why it exists, rather than merely restating its name or signature.
For classes and interfaces:
/**
* This class serves as a factory for HTTP clients, autodiscovering
* the HTTP client if none is passed by the user.
*/
final class HttpClientFactory implements HttpClientFactoryInterface
For methods and functions:
/**
* Gets the callbacks used to customize how objects are serialized
* in the payload of the event.
*
* @return array<string, callable>
*/
public function getClassSerializers(): array
For getters/setters, avoid trivial descriptions like “Gets the X option” or “Sets the X option.” Instead, explain the purpose:
/**
* Gets whether the silenced errors should be captured or not.
*
* @return bool
*/
public function shouldCaptureSilencedErrors(): bool
When implementing interface methods, use the {@inheritdoc} tag while adding any additional context specific to the implementation:
/**
* {@inheritdoc}
*
* This implementation uses PSR-7 compliant request objects.
*/
public function fetchRequest(): ?ServerRequestInterface
Empty docblocks or those that merely repeat the method name add no value and should be either enhanced with meaningful descriptions or removed entirely.
Regularly update testing framework dependencies (PHPUnit, PHPStan, etc.) to maintain compatibility with your project’s PHP version requirements and ensure access to new testing features. When updating dependencies, consider future compatibility needs by using appropriate version constraints.
| For critical testing tools like PHPUnit, evaluate whether version constraints should allow for newer major versions to support upcoming PHP versions (e.g., “^8.5” vs “^8.5 | ^9.0”). |
Example in composer.json:
{
"require-dev": {
"phpunit/phpunit": "^8.5|^9.0", // Allows for PHP 8 testing in the future
"symfony/phpunit-bridge": "^4.3",
"phpstan/phpstan": "^0.11",
"phpstan/phpstan-phpunit": "^0.11"
}
}
Maintaining current testing dependencies helps prevent technical debt and ensures your test suite remains reliable as the project evolves.
Stay informed about API changes in observability libraries like OpenTelemetry, especially during their active development phases. Ensure that both implementation code and documentation are updated to reflect the current API patterns.
When implementing observability tooling:
For example, note how OpenTelemetry’s API changed between versions:
# For older OpenTelemetry 0.4:
from opentelemetry.ext.flask_util import instrument_app
tracer.add_span_processor(span_processor)
# For newer OpenTelemetry versions:
from opentelemetry.ext.flask import instrument_app
trace.tracer_provider().add_span_processor(span_processor)
Maintaining correct configuration is essential for collecting accurate telemetry data. Inconsistent or outdated observability code can result in missing or incorrect metrics, traces, and logs.