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.

Sign up for GitHub

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

Jump to bottom

feat: add span.Warnings to cassandra schema #4217

Closed
wants to merge 1 commit into from
Closed

feat: add span.Warnings to cassandra schema #4217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion examples/hotrod/pkg/tracing/rpcmetrics/observer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import (
"time"

opentracing "github.com/opentracing/opentracing-go"
"github.com/opentracing/opentracing-go/ext"
"github.com/stretchr/testify/assert"
otbridge "go.opentelemetry.io/otel/bridge/opentracing"
"go.opentelemetry.io/otel/sdk/resource"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
semconv "go.opentelemetry.io/otel/semconv/v1.7.0"
"github.com/opentracing/opentracing-go/ext"

u "github.com/jaegertracing/jaeger/internal/metricstest"
)
Expand Down
2 changes: 2 additions & 0 deletions plugin/storage/cassandra/spanstore/dbmodel/converter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ func (c converter) fromDomain(span *model.Span) *Span {
Logs: logs,
Refs: refs,
Process: udtProcess,
Warnings: span.Warnings,
ServiceName: span.Process.ServiceName,
SpanHash: int64(spanHash),
}
Expand Down Expand Up @@ -107,6 +108,7 @@ func (c converter) toDomain(dbSpan *Span) (*model.Span, error) {
Tags: tags,
Logs: logs,
Process: process,
Warnings: dbSpan.Warnings,
}
return span, nil
}
Expand Down
1 change: 1 addition & 0 deletions plugin/storage/cassandra/spanstore/dbmodel/model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type Span struct {
Logs []Log
Refs []SpanRef
Process Process
Warnings []string
ServiceName string
SpanHash int64
}
Expand Down
4 changes: 3 additions & 1 deletion plugin/storage/cassandra/spanstore/reader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,9 @@ func (s *SpanReader) readTraceInSpan(ctx context.Context, traceID dbmodel.TraceI
var refs []dbmodel.SpanRef
var tags []dbmodel.KeyValue
var logs []dbmodel.Log
var warnings []string
retMe := &model.Trace{}
for i.Scan(&traceIDFromSpan, &spanID, &parentID, &operationName, &flags, &startTime, &duration, &tags, &logs, &refs, &dbProcess) {
for i.Scan(&traceIDFromSpan, &spanID, &parentID, &operationName, &flags, &startTime, &duration, &tags, &logs, &refs, &dbProcess, &warnings) {
dbSpan := dbmodel.Span{
TraceID: traceIDFromSpan,
SpanID: spanID,
Expand All @@ -186,6 +187,7 @@ func (s *SpanReader) readTraceInSpan(ctx context.Context, traceID dbmodel.TraceI
Logs: logs,
Refs: refs,
Process: dbProcess,
Warnings: warnings,
ServiceName: dbProcess.ServiceName,
}
span, err := dbmodel.ToDomain(&dbSpan)
Expand Down
5 changes: 3 additions & 2 deletions plugin/storage/cassandra/spanstore/writer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ const (
insertSpan = `
INSERT
INTO traces(trace_id, span_id, span_hash, parent_id, operation_name, flags,
start_time, duration, tags, logs, refs, process)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`
start_time, duration, tags, logs, refs, process, warnings)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`
Comment on lines +39 to +40
Copy link
Member

Choose a reason for hiding this comment

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

This won't work because there is no warning field in the table schema.

Considering how much headache it is to add a new column (need to version schema, invent some backwards compatibility logic, etc.), perhaps a less disruptive solution would be to encode warnings in either tags or logs, with a magic string prefix for tag names, so that they can be recognized at read time and converted back to warnings.

Copy link
Member

Choose a reason for hiding this comment

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

For example, we could create tags like this:

  • $$span.warning.1 = {warning1}
  • $$span.warning.2 = {warning2}

Copy link
Contributor

@utsavoza utsavoza Mar 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Should we be concerned about the length of the warning (string) we receive in the span? Or do we directly encode the entire warning as is?

Copy link
Member

Choose a reason for hiding this comment

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

warnings come from our code, they are not very long. And we generally do not have restrictions on tag value length.

Copy link
Member

Choose a reason for hiding this comment

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

Alternative implementation in #4313


serviceNameIndex = `
INSERT
Expand Down Expand Up @@ -168,6 +168,7 @@ func (s *SpanWriter) writeSpan(span *model.Span, ds *dbmodel.Span) error {
ds.Logs,
ds.Refs,
ds.Process,
ds.Warnings,
)
if err := s.writerMetrics.traces.Exec(mainQuery, s.logger); err != nil {
return s.logError(ds, err, "Failed to insert span", s.logger)
Expand Down