Skip to content

[ES][v2] Implement WriteTraces for v2 #7020

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

Merged
merged 2 commits into from
Apr 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions internal/storage/v2/elasticsearch/tracestore/writer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright (c) 2025 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package tracestore

import (
"context"

"go.opentelemetry.io/collector/pdata/ptrace"

"github.com/jaegertracing/jaeger-idl/model/v1"
cfg "github.com/jaegertracing/jaeger/internal/storage/elasticsearch/config"
"github.com/jaegertracing/jaeger/internal/storage/v1/elasticsearch/spanstore"
)

type TraceWriter struct {
spanWriter spanstore.CoreSpanWriter
}

// NewTraceWriter returns the TraceWriter for use
func NewTraceWriter(p spanstore.SpanWriterParams) *TraceWriter {
return &TraceWriter{
spanWriter: spanstore.NewSpanWriter(p),
}
}

// WriteTraces convert the traces to ES Span model and write into the database
func (t *TraceWriter) WriteTraces(_ context.Context, td ptrace.Traces) error {
dbSpans := ToDBModel(td)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once tagDotReplacement is embedded, this must be replaced by lambda function

for i := 0; i < len(dbSpans); i++ {
span := &dbSpans[i]
t.spanWriter.WriteSpan(model.EpochMicrosecondsAsTime(span.StartTime), span)
Copy link
Member

Choose a reason for hiding this comment

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

I think this is where we should start diverging with v1 implementation. One of the strengths of v2 storage API is that it allows batch writes. v1 API was a single span at a time. So v1 writer had a workaround by running a background batch insert, which also had a problem of not being able to report errors from the writes.

I think we can do better in v2. I don't know if we even need to have a background batching, because this can already configured in the pipeline above the storage. Instead we can treat each payload as already large-enough batch and write all spans in a single request.

}
return nil
}

func (t *TraceWriter) CreateTemplates(spanTemplate, serviceTemplate string, indexPrefix cfg.IndexPrefix) error {
Copy link
Member

Choose a reason for hiding this comment

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

why is this a public function? Who would be calling it?

return t.spanWriter.CreateTemplates(spanTemplate, serviceTemplate, indexPrefix)
}

func (t *TraceWriter) Close() error {
return t.spanWriter.Close()
}
54 changes: 54 additions & 0 deletions internal/storage/v2/elasticsearch/tracestore/writer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Copyright (c) 2025 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

package tracestore

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/collector/pdata/ptrace"

"github.com/jaegertracing/jaeger-idl/model/v1"
cfg "github.com/jaegertracing/jaeger/internal/storage/elasticsearch/config"
"github.com/jaegertracing/jaeger/internal/storage/v1/elasticsearch/spanstore"
"github.com/jaegertracing/jaeger/internal/storage/v1/elasticsearch/spanstore/mocks"
)

func TestTraceWriter_WriteTraces(t *testing.T) {
coreWriter := &mocks.CoreSpanWriter{}
td := ptrace.NewTraces()
resourceSpans := td.ResourceSpans().AppendEmpty()
resourceSpans.Resource().Attributes().PutStr("service.name", "testing-service")
span := resourceSpans.ScopeSpans().AppendEmpty().Spans().AppendEmpty()
span.SetName("op-1")
dbSpan := ToDBModel(td)
writer := TraceWriter{spanWriter: coreWriter}
coreWriter.On("WriteSpan", model.EpochMicrosecondsAsTime(dbSpan[0].StartTime), &dbSpan[0])
err := writer.WriteTraces(context.Background(), td)
require.NoError(t, err)
}

func TestTraceWriter_Close(t *testing.T) {
coreWriter := &mocks.CoreSpanWriter{}
coreWriter.On("Close").Return(nil)
writer := TraceWriter{spanWriter: coreWriter}
err := writer.Close()
require.NoError(t, err)
Copy link
Member

Choose a reason for hiding this comment

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

one-sided test. Wrap it in a loop for _, expErr := range []error{nil, assert.AnError} and check that correct value is returned from Close.

}

func TestTraceWriter_CreateTemplates(t *testing.T) {
coreWriter := &mocks.CoreSpanWriter{}
coreWriter.On("CreateTemplates", "testing-template", "testing-template", cfg.IndexPrefix("testing")).Return(nil)
writer := TraceWriter{spanWriter: coreWriter}
err := writer.CreateTemplates("testing-template", "testing-template", "testing")
require.NoError(t, err)
}

func Test_NewTraceWriter(t *testing.T) {
params := spanstore.SpanWriterParams{}
writer := NewTraceWriter(params)
assert.NotNil(t, writer)
}
Loading