Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(telemetry): Support both new and deprecated OpenTelemetry runtime metrics #999

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ijsong
Copy link
Member

@ijsong ijsong commented Feb 23, 2025

What this PR does

This commit enables Varlog to support both new and deprecated OpenTelemetry
runtime metrics.

Previously, Varlog only supported the deprecated runtime instrumentation. This
change ensures backward compatibility by simultaneously setting the
OTEL_GO_X_DEPRECATED_RUNTIME_METRICS environment variable and using the new
version.

This enhancement allows Varlog to leverage the latest OpenTelemetry features
while maintaining compatibility with older versions, collecting more
comprehensive runtime metrics.

This commit deprecates the error field in AppendResult as a step towards
implementing atomic append operations while maintaining backward compatibility.

Previously, the Append RPC could return partial success/failure results, with
some log entries in a batch being appended successfully and others failing. This
was indicated by the error field in AppendResult.

To support atomic append operations without breaking existing clients, the error
field is deprecated instead of being removed completely. This change allows
clients to continue using the error field for now, but they should be aware that
it will be removed in a future release. Clients should start migrating to the
new atomic append API as soon as possible.

The following changes were made to deprecate the error field:

- The error field in AppendResult is marked as deprecated in the protobuf
  definition.
- The Append RPC implementation no longer populates the error field.

The next step is to implement atomic append operations in the client API. This
will enable clients to append multiple log entries atomically, which will help
to resolve #843.
This PR reduces append wait groups to a single object for each append batch. The
append wait groups are part of the following structs:

- appendContext
- commitWaitTask
- sequenceTask

Previously, multiple append wait groups existed because each log entry in a
batch had its own wait group. With the changes for issue #843, a single append
wait group now corresponds to an entire batch. This PR consolidates the list of
append wait groups into a single append wait group.

TODO: We are still on the road to resolving #843. The rest of the work mostly
involves cleaning up the code base.
This PR removes the totalBytes field from the appendContext struct. It modifies
prepareAppendContext to return totalBytes instead of changing the appendContext
struct. Through this PR, the appendContext becomes slightly simple.
@ijsong ijsong requested a review from hungryjang as a code owner February 23, 2025 12:55
Copy link

codecov bot commented Feb 23, 2025

Codecov Report

Attention: Patch coverage is 86.02151% with 13 lines in your changes missing coverage. Please review.

Project coverage is 79.98%. Comparing base (30645f0) to head (d0cd709).

Files with missing lines Patch % Lines
pkg/util/telemetry/metric_provider.go 0.00% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #999      +/-   ##
==========================================
+ Coverage   79.91%   79.98%   +0.07%     
==========================================
  Files         178      178              
  Lines       21408    21417       +9     
==========================================
+ Hits        17108    17131      +23     
+ Misses       3504     3497       -7     
+ Partials      796      789       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…e metrics

This commit enables Varlog to support both new and deprecated OpenTelemetry
runtime metrics.

Previously, Varlog only supported the deprecated runtime instrumentation. This
change ensures backward compatibility by simultaneously setting the
`OTEL_GO_X_DEPRECATED_RUNTIME_METRICS` environment variable and using the new
version.

This enhancement allows Varlog to leverage the latest OpenTelemetry features
while maintaining compatibility with older versions, collecting more
comprehensive runtime metrics.
@ijsong ijsong force-pushed the new_and_deprecated_opentelemetry_runtime_metrics branch from 3ac7750 to d0cd709 Compare February 24, 2025 07:38
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.

1 participant