Skip to content

Conversation

@Arunodoy18
Copy link

Description

This PR updates documentation comments across the pdata package to provide consistent and clearer messaging about invalid usage of zero-initialized instances, similar to what was done for pcommon.Value in previous changes.

Changes

  • Updated code generation templates (one_of_field.go, one_of_message_value.go) to generate improved comment wording
  • Manually updated pdata/pcommon/value.go comments (12 occurrences)
  • Updated all generated pdata files to use consistent wording

Before vs After

Before:

// Calling this function on zero-initialized Value will cause a panic.
// Calling this function on zero-initialized Value is invalid and will cause a panic.

Motivation
The updated wording makes it clearer that using zero-initialized instances is not just dangerous, but explicitly invalid usage. This improves API documentation clarity and helps developers understand that:

Zero-initialized instances should not be used
The panic is a result of invalid usage, not an edge case
Files Modified
Template files: internal/cmd/pdatagen/internal/pdata/one_of_field.go, [one_of_message_value.go](vscode-file://vscode-app/c:/Users/aruno/AppData/Local/Programs/Microsoft%20VS%20Code/resources/app/out/vs/code/electron-browser/workbench/workbench.html)
Manual update: [value.go](vscode-file://vscode-app/c:/Users/aruno/AppData/Local/Programs/Microsoft%20VS%20Code/resources/app/out/vs/code/electron-browser/workbench/workbench.html)
Generated files: pdata/pmetric/generated_*.go (3 files updated with 13 total occurrences)
Testing
Verified no compilation errors
Consistent comment wording across all pdata types
Future code generation will use the updated templates automatically
Closes:9070

When service.telemetry.metrics.level is set to 'none', the collector
should skip registering process metrics to avoid errors on platforms
where gopsutil is not supported (such as AIX).

This change conditionally registers process metrics only when the
metrics level is not LevelNone, preventing the 'failed to register
process metrics: not implemented yet' error on unsupported platforms.

Fixes regression introduced in v0.136.0 where the check for metrics
level was removed.
Similar to the resolution for pcommon.Value in previous changes, this update
ensures consistent documentation across all pdata types by clarifying that
calling functions on zero-initialized instances is invalid usage.

Changes:
- Updated template files (one_of_field.go, one_of_message_value.go) to generate
  improved comment wording
- Updated pcommon/value.go comments manually
- Updated all generated pdata files to use consistent wording:
  'is invalid and will cause a panic' instead of 'will cause a panic'

This makes it clearer that using zero-initialized instances is not just
dangerous but explicitly invalid usage, improving API documentation clarity.
@Arunodoy18 Arunodoy18 requested review from a team, bogdandrutu and dmitryax as code owners January 1, 2026 10:47
@dmitryax
Copy link
Member

dmitryax commented Jan 5, 2026

@Arunodoy18 please address the CI failures

if err := proctelemetry.RegisterProcessMetrics(srv.telemetrySettings); err != nil {
return nil, fmt.Errorf("failed to register process metrics: %w", err)
// Only register process metrics if metrics telemetry is enabled
if telemetryCfg, ok := cfg.Telemetry.(*otelconftelemetry.Config); !ok || telemetryCfg.Metrics.Level != configtelemetry.LevelNone {
Copy link
Contributor

Choose a reason for hiding this comment

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

this change is unrelated to the comment change, please remove this from the pr and we can get this merged

Copy link
Author

Choose a reason for hiding this comment

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

Yeah sure , Doing this .
Thank you

Copy link
Author

Choose a reason for hiding this comment

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

Hey, Things are been removed , You can Now check for the unrelated changes for merging.
Thank you

Arunodoy18 added a commit to Arunodoy18/opentelemetry-collector that referenced this pull request Jan 6, 2026
…cenarios

This enhancement adds a connection_pool_size configuration option to the OTLP
exporter, enabling multiple gRPC connections with round-robin load balancing.

Key changes:
- Add connection_pool_size config parameter (default: 0, uses 1 connection)
- Implement round-robin load balancing across multiple connections
- Support for 1-256 concurrent gRPC connections
- Backward compatible: default behavior unchanged

This resolves performance issues in high-throughput environments (10K+ spans/sec)
and high-latency network scenarios where a single gRPC connection becomes a
bottleneck.

Also fixes unrelated service.go issue per contributor feedback on PR open-telemetry#14342.
@Arunodoy18
Copy link
Author

Can anyone guide me with the Failures.
rest i will follow up accordingly

@dmathieu
Copy link
Member

dmathieu commented Jan 6, 2026

You need to run make gotidy locally, and possibly make genotelcorecol as well.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants