feat: Tracehandle implementation for tracing in GCSFuse

[thrivikram-karur-g]

Description

Add traceHandle implementation support as custom no-op implementation for tracing in GCSFuse to reduce the impact w.r.t performance or time spent when tracing is disabled.

Link to the issue in case of a bug fix.

b/454481823
b/473717049

Testing details

1. Manual

Manually verified that the fs-ops traces are being exported to google cloud as usual and as expected.

2. Unit tests - modified
3. Integration tests - NA

Any backward incompatible change? If so, please explain.

NA

[thrivikram-karur-g]

/gemini review

[thrivikram-karur-g]

/gemini summary

[gemini-code-assist]

Code Review

This pull request introduces a TraceHandle interface to abstract tracing logic, providing both an OpenTelemetry-based implementation and a no-op implementation. This is a great improvement as it allows tracing to be disabled with minimal performance overhead. The new traceHandle is correctly plumbed through most of the application stack. The code is well-structured, and the use of constants for trace names and attributes is a good practice. I've identified a critical issue in the test setup that prevents tests from running, and a couple of other areas for improvement.

[gemini-code-assist]

Summary of Changes

This pull request significantly refactors the tracing mechanism within GCSFuse by introducing a flexible TraceHandle interface. This abstraction allows the system to dynamically switch between a performance-optimized no-op tracer and a full OpenTelemetry-based tracer, depending on configuration. The change ensures that tracing can be seamlessly integrated across various internal components, providing better observability when enabled, without incurring unnecessary overhead when disabled.

Highlights

• Tracing Abstraction: Introduced a new TraceHandle interface and two implementations: noopTracer for when tracing is disabled to minimize performance impact, and otelTracer for OpenTelemetry-based tracing when enabled.
• TraceHandle Propagation: The TraceHandle is now propagated throughout key components of GCSFuse, including mountWithArgs, BufferedReader, JobManager, FileHandle, ReadManager, GCSReader, and FileCacheReader, ensuring consistent tracing context.
• Refactored Tracing Wrapper: The fs/wrappers/tracing.go component has been updated to utilize the new TraceHandle interface, centralizing how traces are started, ended, and errors are recorded.
• Test Enhancements: Test files have been updated to correctly pass the TraceHandle and to use tracing.NewNoopTracer() where appropriate, improving test robustness and consistency.

Changelog

• cmd/legacy_main.go
  • Imported the tracing package.
  • Modified mountWithArgs function signature to accept tracing.TraceHandle.
  • Initialized traceHandle based on whether tracing is enabled, using tracing.NewNoopTracer() or tracing.NewOtelTracer().
  • Passed the new traceHandle to the mountWithArgs call.
• cmd/mount.go
  • Imported the tracing package.
  • Modified mountWithStorageHandle function signature to accept tracing.TraceHandle.
  • Added TraceHandle field to the ServerConfig struct.
  • Passed the traceHandle to the fs.NewFileSystem call.
• internal/bufferedread/buffered_reader.go
  • Imported the tracing package.
  • Added traceHandle field to BufferedReader and BufferedReaderOptions structs.
  • Passed traceHandle during the creation of NewBufferedReader.
• internal/cache/file/cache_handle_test.go
  • Imported the tracing package.
  • Updated calls to NewCacheHandle in tests to pass tracing.NewNoopTracer().
• internal/cache/file/cache_handler_test.go
  • Imported the tracing package.
  • Updated the downloader.NewJobManager call in initializeCacheHandlerTestArgs to pass tracing.NewNoopTracer().
• internal/cache/file/downloader/downloader.go
  • Imported the tracing package.
  • Added traceHandle field to the JobManager struct.
  • Added traceHandle parameter to NewJobManager and CreateJobIfNotExists functions.
• internal/cache/file/downloader/downloader_test.go
  • Imported the tracing package.
  • Updated the NewJobManager call in setupHelper to pass tracing.NewNoopTracer().
• internal/cache/file/downloader/jm_parallel_downloads_test.go
  • Imported the tracing package.
  • Updated NewJobManager calls in tests to pass tracing.NewNoopTracer().
• internal/cache/file/downloader/job.go
  • Imported the tracing package.
  • Added traceHandle field to the Job struct.
  • Added traceHandle parameter to the NewJob function.
• internal/cache/file/downloader/job_test.go
  • Imported the tracing package.
  • Updated the NewJob call in initJobTest to pass tracing.NewNoopTracer().
• internal/cache/file/downloader/job_testify_test.go
  • Imported the tracing package.
  • Updated the NewJob call in initReadCacheTestifyTest to pass tracing.NewNoopTracer().
• internal/fs/fs.go
  • Imported the tracing package and removed direct go.opentelemetry.io/otel/trace import.
  • Added TraceHandle field to ServerConfig and fileSystem structs.
  • Passed traceHandle to file.NewCacheHandler and handle.NewFileHandle.
  • Replaced the internal maybePropagateTraceContext function with tracing.MaybePropagateTraceContext.
• internal/fs/tracing_test.go
  • Imported the tracing package.
  • Modified newInMemoryExporter to be a global function and updated TestMain to initialize a shared exporter.
  • Updated wrappers.WithTracing calls to pass tracing.NewOtelTracer().
  • Adjusted tests to use globalExporter.Reset() and globalExporter.GetSpans() for span management.
• internal/fs/wrappers/tracing.go
  • Updated imports to use the new internal_trace package.
  • Modified the tracing struct to hold an internal_trace.TraceHandle.
  • Updated the WithTracing function signature to accept an internal_trace.TraceHandle.
  • Refactored invokeWrapped to use traceHandle methods for trace operations (start, end, record error).
• internal/fs/wrappers/tracing_test.go
  • Imported the internal_trace package.
  • Updated the tracing struct initialization to use internal_trace.NewOtelTracer().
• internal/gcsx/client_readers/gcs_reader.go
  • Imported the tracing package.
  • Added TraceHandle field to the GCSReaderConfig struct.
• internal/gcsx/file_cache_reader.go
  • Imported the tracing package.
  • Added traceHandle field to the FileCacheReader struct.
  • Added traceHandle parameter to the NewFileCacheReader function.
• internal/gcsx/file_cache_reader_test.go
  • Imported the tracing package.
  • Updated downloader.NewJobManager and NewFileCacheReader calls in tests to pass tracing.NewNoopTracer().
• internal/gcsx/random_reader.go
  • Imported the tracing package.
  • Added traceHandle field to the randomReader struct.
  • Added traceHandle parameter to the NewRandomReader function.
• internal/gcsx/random_reader_stretchr_test.go
  • Imported the tracing package.
  • Updated downloader.NewJobManager and NewRandomReader calls in tests to pass tracing.NewNoopTracer().
• internal/gcsx/random_reader_test.go
  • Imported the tracing package.
  • Updated downloader.NewJobManager and NewRandomReader calls in tests to pass tracing.NewNoopTracer().
• internal/gcsx/read_manager/read_manager.go
  • Imported the tracing package.
  • Added traceHandle field to the ReadManager struct.
  • Added TraceHandle field to the ReadManagerConfig struct.
  • Passed traceHandle to NewFileCacheReader, NewBufferedReader, and clientReaders.NewGCSReader.
• internal/gcsx/read_manager/read_manager_test.go
  • Imported the tracing package.
  • Updated the downloader.NewJobManager call in readManagerConfig to pass tracing.NewNoopTracer().
• tracing/noop_tracer.go
  • New file: Defines noopTracer struct which implements the TraceHandle interface with no-operation methods, used when tracing is disabled.
• tracing/otel_tracer.go
  • New file: Defines otelTracer struct which implements the TraceHandle interface using OpenTelemetry for actual tracing.
• tracing/span_attributes.go
  • New file: Defines constants for common span attributes like IS_CACHE_HIT and BYTES_READ.
• tracing/span_names.go
  • New file: Defines constants for various span names used across GCSFuse operations for consistent tracing.
• tracing/trace_handle.go
  • New file: Defines the TraceHandle interface, standardizing tracing operations.
• tracing/util.go
  • New file: Defines GCSFuseTracer (OpenTelemetry tracer instance) and MaybePropagateTraceContext utility function for context propagation.

Activity

• thrivikram-karur-g requested a Gemini review.
• thrivikram-karur-g requested a Gemini summary.
• gemini-code-assist[bot] identified a critical issue: TestMain in internal/fs/tracing_test.go was missing m.Run() and os.Exit(m.Run()), preventing tests from executing.
• gemini-code-assist[bot] identified a high-priority issue: The TraceHandle field in GCSReaderConfig was added but not used in NewGCSReader, leading to a lack of trace context propagation.
• gemini-code-assist[bot] suggested a medium-priority improvement: Use tracing.NewNoopTracer() instead of nil for traceHandle in test cases to prevent potential nil pointer panics and ensure consistency.

[codecov]

Codecov Report

❌ Patch coverage is 83.46457% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.97%. Comparing base (3a49292) to head (2328975).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
cmd/legacy_main.go	0.00%	6 Missing ⚠️
tracing/noop_tracer.go	25.00%	6 Missing ⚠️
internal/fs/wrappers/tracing.go	95.77%	3 Missing ⚠️
tracing/otel_tracer.go	83.33%	2 Missing ⚠️
tracing/util.go	71.42%	1 Missing and 1 partial ⚠️
cmd/mount.go	0.00%	1 Missing ⚠️
internal/fs/server.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #4235       +/-   ##
===========================================
+ Coverage        0   82.97%   +82.97%     
===========================================
  Files           0      159      +159     
  Lines           0    19210    +19210     
===========================================
+ Hits            0    15940    +15940     
- Misses          0     2674     +2674     
- Partials        0      596      +596     

Flag	Coverage Δ
unittests	82.97% <83.46%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.