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

fixed the issue that windows cannot contain filenames with colons (replaced ':' with '+') #1700

Merged
merged 16 commits into from
Oct 25, 2022
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ query_frontend:
* [ENHANCEMENT] Vulture now has improved distribution of the random traces it searches. [#1763](https://github.com/grafana/tempo/pull/1763) (@rfratto)
* [ENHANCEMENT] Upgrade opentelemetry-proto submodule to v0.18.0 [#1754](https://github.com/grafana/tempo/pull/1754) (@mapno)
Internal types are updated to use `scope` instead of `instrumentation_library`. This is a breaking change in trace by ID queries if JSON is requested.
* [BUGFIX] New wal file separator '+' for the NTFS filesystem and backward compatibility with the old separator ':' [#1700](https://github.com/grafana/tempo/pull/1700) (@kilian-kier)
* [ENHANCEMENT] metrics-generator: extract `status_message` field from spans [#1786](https://github.com/grafana/tempo/pull/1786), [#1794](https://github.com/grafana/tempo/pull/1794) (@stoewer)
* [ENHANCEMENT] metrics-generator: handle collisions between user defined and default dimensions [#1794](https://github.com/grafana/tempo/pull/1794) (@stoewer)
* [ENHANCEMENT] distributor: Log span names when `distributor.log_received_spans.include_all_attributes` is on [#1790](https://github.com/grafana/tempo/pull/1790) (@suraciii)
Expand All @@ -72,6 +73,7 @@ Internal types are updated to use `scope` instead of `instrumentation_library`.
* [BUGFIX] Fix parquet search bug fix on http.status_code that may cause incorrect results to be returned [#1799](https://github.com/grafana/tempo/pull/1799) (@mdisibio)
* [BUGFIX] Fix failing SearchTagValues endpoint after startup [#1813](https://github.com/grafana/tempo/pull/1813) (@stoewer)


## v1.5.0 / 2022-08-17

* [CHANGE] metrics-generator: Changed added metric label `instance` to `__metrics_gen_instance` to reduce collisions with custom dimensions. [#1439](https://github.com/grafana/tempo/pull/1439) (@joe-elliott)
Expand Down
36 changes: 29 additions & 7 deletions tempodb/encoding/v2/append_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package v2

import (
"context"
"errors"
"fmt"
"math"
"os"
Expand Down Expand Up @@ -38,8 +39,8 @@ type v2AppendBlock struct {
once sync.Once
}

func newAppendBlock(id uuid.UUID, tenantID string, filepath string, e backend.Encoding, dataEncoding string, ingestionSlack time.Duration) (common.WALBlock, error) {
if strings.ContainsRune(dataEncoding, ':') ||
func newAppendBlock(id uuid.UUID, tenantID string, filepath string, e backend.Encoding, dataEncoding string, ingestionSlack time.Duration) (*v2AppendBlock, error) {
if strings.ContainsRune(dataEncoding, ':') || strings.ContainsRune(dataEncoding, '+') ||
len([]rune(dataEncoding)) > maxDataEncodingLength {
return nil, fmt.Errorf("dataEncoding %s is invalid", dataEncoding)
}
Expand Down Expand Up @@ -261,15 +262,29 @@ func (a *v2AppendBlock) Fetch(context.Context, traceql.FetchSpansRequest) (trace
}

func (a *v2AppendBlock) fullFilename() string {
filename := a.fullFilenameSeparator("+")
_, e1 := os.Stat(filename)
if errors.Is(e1, os.ErrNotExist) {
filenameWithOldSeparator := a.fullFilenameSeparator(":")
_, e2 := os.Stat(filenameWithOldSeparator)
if !errors.Is(e2, os.ErrNotExist) {
filename = filenameWithOldSeparator
}
}

return filename
}

func (a *v2AppendBlock) fullFilenameSeparator(separator string) string {
if a.meta.Version == "v0" {
return filepath.Join(a.filepath, fmt.Sprintf("%v:%v", a.meta.BlockID, a.meta.TenantID))
return filepath.Join(a.filepath, fmt.Sprintf("%v%v%v", a.meta.BlockID, separator, a.meta.TenantID))
}

var filename string
if a.meta.DataEncoding == "" {
filename = fmt.Sprintf("%v:%v:%v:%v", a.meta.BlockID, a.meta.TenantID, a.meta.Version, a.meta.Encoding)
filename = fmt.Sprintf("%v%v%v%v%v%v%v", a.meta.BlockID, separator, a.meta.TenantID, separator, a.meta.Version, separator, a.meta.Encoding)
} else {
filename = fmt.Sprintf("%v:%v:%v:%v:%v", a.meta.BlockID, a.meta.TenantID, a.meta.Version, a.meta.Encoding, a.meta.DataEncoding)
filename = fmt.Sprintf("%v%v%v%v%v%v%v%v%v", a.meta.BlockID, separator, a.meta.TenantID, separator, a.meta.Version, separator, a.meta.Encoding, separator, a.meta.DataEncoding)
}

return filepath.Join(a.filepath, filename)
Expand Down Expand Up @@ -311,9 +326,16 @@ func (a *v2AppendBlock) adjustTimeRangeForSlack(start uint32, end uint32, additi
}

// ParseFilename returns (blockID, tenant, version, encoding, dataEncoding, error).
// Example: "00000000-0000-0000-0000-000000000000:1:v2:snappy:v1"
// Example: "00000000-0000-0000-0000-000000000000+1+v2+snappy+v1"
// Example with old separator: "00000000-0000-0000-0000-000000000000:1:v2:snappy:v1"
func ParseFilename(filename string) (uuid.UUID, string, string, backend.Encoding, string, error) {
splits := strings.Split(filename, ":")
var splits []string
if strings.Contains(filename, "+") {
splits = strings.Split(filename, "+")
} else {
// backward-compatibility with the old separator
splits = strings.Split(filename, ":")
}

if len(splits) != 4 && len(splits) != 5 {
return uuid.UUID{}, "", "", backend.EncNone, "", fmt.Errorf("unable to parse %s. unexpected number of segments", filename)
Expand Down
136 changes: 111 additions & 25 deletions tempodb/encoding/v2/append_block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,87 +31,87 @@ func TestFullFilename(t *testing.T) {
meta: backend.NewBlockMeta("foo", uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), "v0", backend.EncNone, ""),
filepath: "/blerg",
},
expected: "/blerg/123e4567-e89b-12d3-a456-426614174000:foo",
expected: "/blerg/123e4567-e89b-12d3-a456-426614174000+foo",
},
{
name: "ez-mode",
b: &v2AppendBlock{
meta: backend.NewBlockMeta("foo", uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), "v1", backend.EncNone, ""),
filepath: "/blerg",
},
expected: "/blerg/123e4567-e89b-12d3-a456-426614174000:foo:v1:none",
expected: "/blerg/123e4567-e89b-12d3-a456-426614174000+foo+v1+none",
},
{
name: "nopath",
b: &v2AppendBlock{
meta: backend.NewBlockMeta("foo", uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), "v1", backend.EncNone, ""),
filepath: "",
},
expected: "123e4567-e89b-12d3-a456-426614174000:foo:v1:none",
expected: "123e4567-e89b-12d3-a456-426614174000+foo+v1+none",
},
{
name: "gzip",
b: &v2AppendBlock{
meta: backend.NewBlockMeta("foo", uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), "v2", backend.EncGZIP, ""),
filepath: "",
},
expected: "123e4567-e89b-12d3-a456-426614174000:foo:v2:gzip",
expected: "123e4567-e89b-12d3-a456-426614174000+foo+v2+gzip",
},
{
name: "lz41M",
b: &v2AppendBlock{
meta: backend.NewBlockMeta("foo", uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), "v2", backend.EncLZ4_1M, ""),
filepath: "",
},
expected: "123e4567-e89b-12d3-a456-426614174000:foo:v2:lz4-1M",
expected: "123e4567-e89b-12d3-a456-426614174000+foo+v2+lz4-1M",
},
{
name: "lz4256k",
b: &v2AppendBlock{
meta: backend.NewBlockMeta("foo", uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), "v2", backend.EncLZ4_256k, ""),
filepath: "",
},
expected: "123e4567-e89b-12d3-a456-426614174000:foo:v2:lz4-256k",
expected: "123e4567-e89b-12d3-a456-426614174000+foo+v2+lz4-256k",
},
{
name: "lz4M",
b: &v2AppendBlock{
meta: backend.NewBlockMeta("foo", uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), "v2", backend.EncLZ4_4M, ""),
filepath: "",
},
expected: "123e4567-e89b-12d3-a456-426614174000:foo:v2:lz4",
expected: "123e4567-e89b-12d3-a456-426614174000+foo+v2+lz4",
},
{
name: "lz64k",
b: &v2AppendBlock{
meta: backend.NewBlockMeta("foo", uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), "v2", backend.EncLZ4_64k, ""),
filepath: "",
},
expected: "123e4567-e89b-12d3-a456-426614174000:foo:v2:lz4-64k",
expected: "123e4567-e89b-12d3-a456-426614174000+foo+v2+lz4-64k",
},
{
name: "snappy",
b: &v2AppendBlock{
meta: backend.NewBlockMeta("foo", uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), "v2", backend.EncSnappy, ""),
filepath: "",
},
expected: "123e4567-e89b-12d3-a456-426614174000:foo:v2:snappy",
expected: "123e4567-e89b-12d3-a456-426614174000+foo+v2+snappy",
},
{
name: "zstd",
b: &v2AppendBlock{
meta: backend.NewBlockMeta("foo", uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), "v2", backend.EncZstd, ""),
filepath: "",
},
expected: "123e4567-e89b-12d3-a456-426614174000:foo:v2:zstd",
expected: "123e4567-e89b-12d3-a456-426614174000+foo+v2+zstd",
},
{
name: "data encoding",
b: &v2AppendBlock{
meta: backend.NewBlockMeta("foo", uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), "v1", backend.EncNone, "dataencoding"),
filepath: "/blerg",
},
expected: "/blerg/123e4567-e89b-12d3-a456-426614174000:foo:v1:none:dataencoding",
expected: "/blerg/123e4567-e89b-12d3-a456-426614174000+foo+v1+none+dataencoding",
},
}

Expand Down Expand Up @@ -191,7 +191,7 @@ func TestPartialBlock(t *testing.T) {
}

// append garbage data
v2Block := block.(*v2AppendBlock)
v2Block := block
garbo := make([]byte, 100)
_, err = rand.Read(garbo)
require.NoError(t, err)
Expand Down Expand Up @@ -237,7 +237,7 @@ func TestParseFilename(t *testing.T) {
expectError bool
}{
{
name: "version, enc snappy and dataencoding",
name: "version, enc snappy and dataencoding (old separator)",
filename: "123e4567-e89b-12d3-a456-426614174000:foo:v2:snappy:dataencoding",
expectUUID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"),
expectTenant: "foo",
Expand All @@ -246,7 +246,7 @@ func TestParseFilename(t *testing.T) {
expectedDataEncoding: "dataencoding",
},
{
name: "version, enc none and dataencoding",
name: "version, enc none and dataencoding (old separator)",
filename: "123e4567-e89b-12d3-a456-426614174000:foo:v2:none:dataencoding",
expectUUID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"),
expectTenant: "foo",
Expand All @@ -255,7 +255,7 @@ func TestParseFilename(t *testing.T) {
expectedDataEncoding: "dataencoding",
},
{
name: "empty dataencoding",
name: "empty dataencoding (old separator)",
filename: "123e4567-e89b-12d3-a456-426614174000:foo:v2:snappy",
expectUUID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"),
expectTenant: "foo",
Expand All @@ -264,7 +264,7 @@ func TestParseFilename(t *testing.T) {
expectedDataEncoding: "",
},
{
name: "empty dataencoding with semicolon",
name: "empty dataencoding with semicolon (old separator)",
filename: "123e4567-e89b-12d3-a456-426614174000:foo:v2:snappy:",
expectUUID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"),
expectTenant: "foo",
Expand All @@ -273,7 +273,7 @@ func TestParseFilename(t *testing.T) {
expectedDataEncoding: "",
},
{
name: "path fails",
name: "path fails (old separator)",
filename: "/blerg/123e4567-e89b-12d3-a456-426614174000:foo",
expectError: true,
},
Expand All @@ -288,48 +288,134 @@ func TestParseFilename(t *testing.T) {
expectError: true,
},
{
name: "bad uuid",
name: "bad uuid (old separator)",
filename: "123e4:foo",
expectError: true,
},
{
name: "no tenant",
name: "no tenant (old separator)",
filename: "123e4567-e89b-12d3-a456-426614174000:",
expectError: true,
},
{
name: "no version",
name: "no version (old separator)",
filename: "123e4567-e89b-12d3-a456-426614174000:test::none",
expectError: true,
},
{
name: "wrong splits - 6",
name: "wrong splits - 6 (old separator)",
filename: "123e4567-e89b-12d3-a456-426614174000:test:test:test:test:test",
expectError: true,
},
{
name: "wrong splits - 3",
name: "wrong splits - 3 (old separator)",
filename: "123e4567-e89b-12d3-a456-426614174000:test:test",
expectError: true,
},
{
name: "wrong splits - 1 (old separator)",
filename: "123e4567-e89b-12d3-a456-426614174000",
expectError: true,
},
{
name: "bad encoding (old separator)",
filename: "123e4567-e89b-12d3-a456-426614174000:test:v1:asdf",
expectError: true,
},
{
name: "ez-mode old format (old separator)",
filename: "123e4567-e89b-12d3-a456-426614174000:foo",
expectError: true,
},
{
name: "deprecated version (old separator)",
filename: "123e4567-e89b-12d3-a456-426614174000:foo:v1:snappy",
expectError: true,
},
{
name: "version, enc snappy and dataencoding",
filename: "123e4567-e89b-12d3-a456-426614174000+foo+v2+snappy+dataencoding",
expectUUID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"),
expectTenant: "foo",
expectedVersion: "v2",
expectedEncoding: backend.EncSnappy,
expectedDataEncoding: "dataencoding",
},
{
name: "version, enc none and dataencoding",
filename: "123e4567-e89b-12d3-a456-426614174000+foo+v2+none+dataencoding",
expectUUID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"),
expectTenant: "foo",
expectedVersion: "v2",
expectedEncoding: backend.EncNone,
expectedDataEncoding: "dataencoding",
},
{
name: "empty dataencoding",
filename: "123e4567-e89b-12d3-a456-426614174000+foo+v2+snappy",
expectUUID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"),
expectTenant: "foo",
expectedVersion: "v2",
expectedEncoding: backend.EncSnappy,
expectedDataEncoding: "",
},
{
name: "empty dataencoding with plus sign",
filename: "123e4567-e89b-12d3-a456-426614174000+foo+v2+snappy+",
expectUUID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"),
expectTenant: "foo",
expectedVersion: "v2",
expectedEncoding: backend.EncSnappy,
expectedDataEncoding: "",
},
{
name: "path fails",
filename: "/blerg/123e4567-e89b-12d3-a456-426614174000+foo",
expectError: true,
},
{
name: "bad uuid",
filename: "123e4+foo",
expectError: true,
},
{
name: "no tenant",
filename: "123e4567-e89b-12d3-a456-426614174000+",
expectError: true,
},
{
name: "no version",
filename: "123e4567-e89b-12d3-a456-426614174000+test++none",
expectError: true,
},
{
name: "wrong splits - 6",
filename: "123e4567-e89b-12d3-a456-426614174000+test+test+test+test+test",
expectError: true,
},
{
name: "wrong splits - 3",
filename: "123e4567-e89b-12d3-a456-426614174000+test+test",
expectError: true,
},
{
name: "wrong splits - 1",
filename: "123e4567-e89b-12d3-a456-426614174000",
expectError: true,
},
{
name: "bad encoding",
filename: "123e4567-e89b-12d3-a456-426614174000:test:v1:asdf",
filename: "123e4567-e89b-12d3-a456-426614174000+test+v1+asdf",
expectError: true,
},
{
name: "ez-mode old format",
filename: "123e4567-e89b-12d3-a456-426614174000:foo",
filename: "123e4567-e89b-12d3-a456-426614174000+foo",
expectError: true,
},
{
name: "deprecated version",
filename: "123e4567-e89b-12d3-a456-426614174000:foo:v1:snappy",
filename: "123e4567-e89b-12d3-a456-426614174000+foo+v1+snappy",
expectError: true,
},
}
Expand Down
2 changes: 1 addition & 1 deletion tempodb/wal/wal.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func (w *WAL) NewFile(blockid uuid.UUID, tenantid string, dir string) (*os.File,
}

// blockID, tenantID, version, encoding (compression), dataEncoding
filename := fmt.Sprintf("%v:%v:%v:%v:%v", blockid, tenantid, walFileVersion, w.c.SearchEncoding, "")
filename := fmt.Sprintf("%v+%v+%v+%v+%v", blockid, tenantid, walFileVersion, w.c.SearchEncoding, "")
file, err := os.OpenFile(filepath.Join(p, filename), os.O_CREATE|os.O_RDWR, 0644)
if err != nil {
return nil, backend.EncNone, err
Expand Down
Loading