Refactor: Centralize Observability in MongoDB & ArangoDB Modules

[akshat-kumar-singhal]

Description:

• Refactored observability code in MongoDB and ArangoDB modules by introducing a centralized pkg/gofr/datasource/observability package
• Restructured AddTrace and OperationStats methods to reduce function parameters and improve readability
• Replaced separate logger, metrics, tracer fields with a single instrumentation field in both ArangoDB and MongoDB datasources
• Added new SetLogger(), SetMetrics(), SetTracer() methods while keeping deprecated UseLogger(), UseMetrics(), UseTracer() for backward compatibility
• Enhanced QueryLog struct with SetDuration(), GetOperation(), and Meta() helper methods
• Marked provider interfaces in container/datasources.go as deprecated in favor of datasource.Observable pattern
• Added comprehensive test coverage for the new observability package, MongoDB, and ArangoDB modules
• Reorganized mocks into dedicated folders for better test organization

Breaking Changes (if applicable):
There are no breaking changes in this, however some deprecations/ cleanups/ standardisations:

• Provider interfaces (MongoProvider, ArangoDBProvider, etc.) are now deprecated; users should migrate to datasource.Observable
• Internal method signatures changed (addTrace → instrumentation.AddTrace, sendOperationStats → instrumentation.OperationStats)
• Removed standalone metrics.go, mock_logger.go, and mock_metrics.go files from arangodb and mongo packages (consolidated into observability package)
• Datastore Histograms (app_arango_stats and app_mongo_stats) to now have standard set of labels - added additional label hostname to app_arango_stats in addition to endpoint with the same value for backward compatibility purposes. endpoint label to be deprecated. Note: since both hostname and endpoint have the same value, it would not cause an increase in the cardinality.

Additional Information:

• New module pkg/gofr/datasource/observability with its own go.mod and go.sum

Checklist:

• I have formatted my code using goimport and golangci-lint.
• All new code is covered by unit tests.
• This PR does not decrease the overall code coverage.
• I have reviewed the code comments and documentation for clarity.

[coolwednesday]

Hey, thanks for putting this together — I can see the intent behind centralizing observability and it's clear a lot of thought went into it.

However, I'd like you to reconsider the approach here. I have a few concerns:

Cross-module coupling

Previously, each datasource (mongo, arangodb, etc.) defined its own Logger and Metrics interfaces locally via duck typing — fully independent, zero shared dependencies. This PR introduces a shared observability module that all datasource modules must now import, which means:

• If observability interfaces change (e.g., adding a method to Logger, Metrics, or ObservableQuery), every dependent datasource module must be updated and released in lockstep.
• Third-party datasource authors now take on a GoFr-internal dependency they didn't need before.

The ObservableQuery interface is too opinionated

It forces every datasource's QueryLog to conform to SetDuration, GetOperation, GetMetricLabels, GetTraceLabels. Different datasources have different observability needs — this abstraction becomes a lowest common denominator.

The defer bug fix doesn't require this scope

The original bug (defer placed after error check, span leak, wrong timing) is valid and worth fixing, but the fix is ~2-3 lines per method — move defer to the top, capture time.Now() before the operation. A simple helper in each datasource achieves the same structural safety:

func startOperation(ctx context.Context, tracer trace.Tracer, spanName string,
    attrs map[string]string) (context.Context, trace.Span, time.Time) {
    startTime := time.Now()
    if tracer == nil {
        return ctx, nil, startTime
    }
    ctx, span := tracer.Start(ctx, spanName)
    for k, v := range attrs {
        span.SetAttributes(attribute.String(k, v))
    }
    return ctx, span, startTime
}

func endOperation(span trace.Span, startTime time.Time, durationKey string,
    logger Logger, ql *QueryLog, metrics Metrics, histogram string, labels ...string) {
    duration := time.Since(startTime).Microseconds()
    ql.Duration = duration
    logger.Debug(ql)
    metrics.RecordHistogram(context.Background(), histogram, float64(duration)/1e6, labels...)
    if span != nil {
        span.SetAttributes(attribute.Int64(durationKey, duration))
        span.End()
    }
}

Yes, this is duplicated across mongo, arangodb, etc. But:

• It's ~20 lines of straightforward code
• Each datasource can tweak it freely
• Zero cross-module dependencies
• Zero interfaces
• Zero new Go modules
• The defer bug is still fixed

What I think is worth keeping

The instrumentDatasource() consolidation in external_db.go is a genuine improvement — the old code had 15+ identical UseLogger/UseMetrics/UseTracer/Connect blocks. But that can be a private helper in the gofr package using anonymous type assertions, no separate module needed:

func (a *App) instrumentDatasource(ds any, tracerName string) {
    if l, ok := ds.(interface{ UseLogger(any) }); ok {
        l.UseLogger(a.Logger())
    }
    if m, ok := ds.(interface{ UseMetrics(any) }); ok {
        m.UseMetrics(a.Metrics())
    }
    if t, ok := ds.(interface{ UseTracer(any) }); ok {
        t.UseTracer(otel.GetTracerProvider().Tracer(tracerName))
    }
    if c, ok := ds.(interface{ Connect() }); ok {
        c.Connect()
    }
}

Summary

A whole module + interfaces + noop implementations + mocks for what amounts to "start timer, start span, defer end" feels like over-engineering. I'd suggest:

1. Fix the defer bug directly in each affected datasource (small, targeted PR).
2. Consolidate external_db.go wiring with a private helper (separate PR).
3. Keep datasource observability local — each module owns its own logging, metrics, and tracing decisions.

Happy to discuss further if you see it differently!

[akshat-kumar-singhal]

It forces every datasource's QueryLog to conform to SetDuration, GetOperation, GetMetricLabels, GetTraceLabels. Different datasources have different observability needs — this abstraction becomes a lowest common denominator.

We could split the Instrumenter interface into 2 parts - initialisation (SetLogger/SetMetrics/SetTracer) and tracking (Logf/ Debugf/ etc) - this way an external datasource implementing the Instrumenter part 1 interface would get the observability tools.
The existing datasources implemented within the gofr ecosystem, albeit as an independent module benefit from the standardisation - unless we have a strong use case of these external modules being actually used independently without gofr.

If observability interfaces change (e.g., adding a method to Logger, Metrics, or ObservableQuery), every dependent datasource module must be updated and released in lockstep.

Breaking change in the interface won't be done unless it's a major release.

The ObservableQuery interface is too opinionated

The concern is understandable - it however should get addressed by splitting the Instrumenter interface - data services interested to use the common capability from the observability package would continue to do it.
PS: Gofr is opinionated 😄

Keep datasource observability local — each module owns its own logging, metrics, and tracing decisions.

Keeping it local would remove the standardisation - this goes against the idea of an opinionated framework.