Skip to content
Draft
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
195 changes: 195 additions & 0 deletions integrationservertest/apm-version-bug_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
package integrationservertest

import (
"errors"
"fmt"
"regexp"
"testing"

"github.com/elastic/apm-server/integrationservertest/internal/ech"
"github.com/elastic/go-elasticsearch/v8/typedapi/types"
)

// See https://github.com/elastic/ingest-dev/issues/5701
// Available tests:
// TestAPMResourcesVersionBug/8.16.2_to_8.17.8
// TestAPMResourcesVersionBug/8.17.7_to_8.17.8
// TestAPMResourcesVersionBug/8.17.8_to_8.18.3
// TestAPMResourcesVersionBug/8.18.2_to_8.18.3
// TestAPMResourcesVersionBug/8.18.3_to_8.19.0
func TestAPMResourcesVersionBug(t *testing.T) {
config, err := parseConfig("upgrade-config.yaml")
if err != nil {
t.Fatal(err)
}

zeroEventIngestedDocs := checkFieldExistsInDocsStep{
dataStreamName: "traces-apm-default",
fieldName: "event.ingested",
checkFn: func(i int64) bool { return i == 0 },
}
noEventIngestedInMappings := checkMappingStep{
datastreamname: "traces-apm-default",
indexName: regexp.MustCompile(".ds-traces-apm-default-[0-9.]+-000001"),
checkFn: func(mappings types.TypeMapping) error {
if !hasNestedField(mappings, "event.ingested") {
return nil
}
return errors.New("there should be no event.ingested here")
},
}
someEventIngestedDocs := checkFieldExistsInDocsStep{
dataStreamName: "traces-apm-default",
fieldName: "event.ingested",
}

// this wraps the usual steps build to add additional checks in between.
// Expect one upgrade, no ingested docs before, some after.
oneUpgradeZeroThenSome := func(t *testing.T, versions ech.Versions, config upgradeTestConfig) []testStep {
steps := buildTestSteps(t, versions, config, false)
return []testStep{
steps[0], // create
steps[1], // ingest
zeroEventIngestedDocs,
noEventIngestedInMappings,
steps[2], // upgrade
steps[3], // ingest
someEventIngestedDocs,
checkMappingStep{
datastreamname: "traces-apm-default",
indexName: regexp.MustCompile(".ds-traces-apm-default-[0-9.]+-000002"),
checkFn: func(mappings types.TypeMapping) error {
if hasNestedField(mappings, "event.ingested") {
return nil
}
return fmt.Errorf("there should be an event.ingested here")
},
},
}
}

// this wraps the usual steps build to add additional checks in between.
// Expect two upgrades, some ingested docs after the first upgrade, some after the second.
twoUpgradesSomeThenSome := func(t *testing.T, versions ech.Versions, config upgradeTestConfig) []testStep {
steps := buildTestSteps(t, versions, config, false)
howmany := int64(0)
return []testStep{
steps[0], // create
steps[1], // ingest
steps[2], // upgrade
steps[3], // ingest
checkFieldExistsInDocsStep{
dataStreamName: "traces-apm-default",
fieldName: "event.ingested",
checkFn: func(i int64) bool {
if i == 0 {
return false
}
// Store howmany to cross check it's higher after next upgrade/ingest cycle.
howmany = i
return true
},
},
checkMappingStep{
datastreamname: "traces-apm-default",
indexName: regexp.MustCompile(".ds-traces-apm-default-[0-9.]+-000002"),
checkFn: func(mappings types.TypeMapping) error {
if hasNestedField(mappings, "event.ingested") {
return nil
}
return fmt.Errorf("there should be an event.ingested here")
},
},
steps[4], // upgrade
steps[5], // upgrade
checkFieldExistsInDocsStep{
dataStreamName: "traces-apm-default",
fieldName: "event.ingested",
checkFn: func(i int64) bool {
if i == 0 {
return false
}
if i <= howmany {
return false
}
return true
},
},
checkMappingStep{
datastreamname: "traces-apm-default",
indexName: regexp.MustCompile(".ds-traces-apm-default-[0-9.]+-000003"),
checkFn: func(mappings types.TypeMapping) error {
if hasNestedField(mappings, "event.ingested") {
return nil
}
return fmt.Errorf("there should be an event.ingested here")
},
},
}
}

version8162 := vsCache.GetLatestVersionOrSkip(t, "8.16.2")
version8177 := vsCache.GetLatestVersionOrSkip(t, "8.17.7")
version8178 := vsCache.GetLatestBCOrSkip(t, "8.17.8") // latest 8.17 as of today
Copy link
Contributor

Choose a reason for hiding this comment

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

This will skip the whole test entirely if the BC does not exist. Since both GetLatestVersionOrSkip and GetLatestBCOrSkip are not used currently, you can update them to achieve what you need instead.

version8182 := vsCache.GetLatestVersionOrSkip(t, "8.18.2")
version8183 := vsCache.GetLatestBCOrSkip(t, "8.18.3") // latest 8.18 as of today
// version8190 := vsCache.GetLatestBCOrSkip(t, "8.19.0") // latest 8.19 as of today
// the event.ingested change was introduced in 8.16.3, as
// it only shows if we upgrade a cluster from a version with
// the bug we need to start from 8.16.2.
start := version8162

t.Run("8.16.2 to 8.17.8", func(t *testing.T) {
versions := []ech.Version{start, version8177}
runner := testStepsRunner{
Target: *target,
Steps: oneUpgradeZeroThenSome(t, versions, config),
}
runner.Run(t)
})

t.Run("8.17.7 to 8.17.8", func(t *testing.T) {
versions := []ech.Version{start, version8177, version8178}
runner := testStepsRunner{
Target: *target,
Steps: twoUpgradesSomeThenSome(t, versions, config),
}
runner.Run(t)
})

t.Run("8.17.8 to 8.18.3", func(t *testing.T) {
versions := []ech.Version{version8178, version8183}
runner := testStepsRunner{
Target: *target,
Steps: buildTestSteps(t, versions, config, false),
}
runner.Run(t)
})

t.Run("8.18.2 to 8.18.3", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If this includes start, shouldn't it be "8.16.2 to 8.17.x to 8.18.x"? Also vsCache.GetLatestSnapshot gets the latest patch for the minor if you specify x.y, so the description will be inaccurate when new patches come out.

Copy link
Member Author

Choose a reason for hiding this comment

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

I named this after our test cases for this bug. I don't think we want to merge this PR as it is, with all these test cases for all these versions. This is now useful to ensure our fixes are working but I would make it more general if we want to include it as our regular testing.

I think a necessary test case to add is an upgrade test: from the "previous previous" minor (ie in this case 8.16.2) to the previous minor and than to the current minor. Such a test could find issues that only appears with an upgrade, a scenario we don't yet cover.

versions := []ech.Version{start, version8182, version8183}
runner := testStepsRunner{
Target: *target,
Steps: twoUpgradesSomeThenSome(t, versions, config),
}
runner.Run(t)
})

t.Run("8.18.3 to 8.19.0", func(t *testing.T) {
// FIXME: use 8.19.0 BC when it becomes available.
start := vsCache.GetLatestSnapshot(t, "8.16.2")
version8183 := vsCache.GetLatestSnapshot(t, "8.18.3")
version8190 := vsCache.GetLatestSnapshot(t, "8.19.0")
versions := []ech.Version{start, version8183, version8190}
steps := twoUpgradesSomeThenSome(t, versions, config)
steps = append(steps, checkFieldExistsInDocsStep{
dataStreamName: "traces-apm-default",
fieldName: "event.success_count",
})
runner := testStepsRunner{
Target: *target,
Steps: steps,
}
runner.Run(t)
})
}
152 changes: 152 additions & 0 deletions integrationservertest/buglib.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
package integrationservertest

import (
"context"
"errors"
"fmt"
"regexp"
"strings"
"testing"

"github.com/elastic/go-elasticsearch/v8"
"github.com/elastic/go-elasticsearch/v8/typedapi/core/count"
"github.com/elastic/go-elasticsearch/v8/typedapi/types"
)

// hasNestedField checks if a nested field (dot-separated path) exists in the properties map.
func hasNestedField(mappings types.TypeMapping, fieldPath string) bool {
parts := strings.Split(fieldPath, ".")
current := mappings.Properties
// fmt.Println("parts", parts)
// fmt.Println("curre", current)
for i, part := range parts {
// Check if the part exists in the current properties map
val, exists := current[part]
// fmt.Println("val", val, "parts", i == len(parts)-1)
if !exists {
return false
}
// If this is the last part, the field exists
if i == len(parts)-1 {
return true
}
// Try to descend into sub-properties (for nested/object fields)
fieldMap, ok := val.(*types.ObjectProperty)
if !ok {
return false
}
// fmt.Println("fieldMap", fieldMap)
subProps := fieldMap.Properties
current = subProps
}
return false
}

type checkMappingStep struct {
datastreamname string
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
datastreamname string
dataStreamName string

indexName *regexp.Regexp
checkFn func(mappings types.TypeMapping) error
}

func (c checkMappingStep) Step(t *testing.T, ctx context.Context, e *testStepEnv) {
t.Logf("------ check data stream mappings in %s ------", e.currentVersion())
t.Logf("checking data stream mapping on %s", c.datastreamname)
err := c.checkDataStreamMappings(t, ctx, e.esc.TypedClient())
if err != nil {
t.Log(err)
t.Fail()
}
t.Log("success")
}

func (c checkMappingStep) checkDataStreamMappings(t *testing.T, ctx context.Context, es *elasticsearch.TypedClient) error {
dataStreamName := c.datastreamname

info, err := es.Indices.GetDataStream().Name(dataStreamName).Do(ctx)
if err != nil {
return fmt.Errorf("Error getting data stream info: %s", err)
}
is := []string{}
for _, v := range info.DataStreams[0].Indices {
is = append(is, v.IndexName)
}
t.Logf("indices: %s", is)

var backingIndex string
for _, v := range info.DataStreams[0].Indices {
if c.indexName.Match([]byte(v.IndexName)) {
backingIndex = v.IndexName
}
}
if backingIndex == "" {
// collect available indices for easier troubleshooting
indices := []string{}
for _, v := range info.DataStreams[0].Indices {
indices = append(indices, v.IndexName)
}
return errors.New(fmt.Sprintf("no index matches the filter regexp; filter=%s indices=%s", c.indexName, indices))
}

mappingRes, err := es.Indices.GetMapping().Index(backingIndex).Do(ctx)
if err != nil {
return fmt.Errorf("Error getting mapping: %w", err)
}

indexMappingRecord, ok := mappingRes[backingIndex]
if !ok {
return errors.New(fmt.Sprintf("Backing index %s not found in mapping", backingIndex))
}

if err := c.checkFn(indexMappingRecord.Mappings); err != nil {
return fmt.Errorf("mapping check failed: %w", err)
}

return nil
}

type checkFieldExistsInDocsStep struct {
dataStreamName string
fieldName string
// checkFn can be used to customize the check this step performs on the returned
// doc count. By default it checks if the vlaue is greater than 0.
checkFn func(i int64) bool
}

func (s checkFieldExistsInDocsStep) Step(t *testing.T, ctx context.Context, e *testStepEnv) {
t.Logf("------ check data stream docs with field %s in %s ------", s.fieldName, e.currentVersion())
count, err := getDocCountWithField(ctx, e.esc.TypedClient(), s.dataStreamName, s.fieldName)
if err != nil {
t.Log(err)
t.Fail()
}
t.Logf("documents in data stream %s with '%s': %d\n", s.dataStreamName, s.fieldName, count)

if s.checkFn == nil {
s.checkFn = func(i int64) bool { return i > 0 }
}

if !s.checkFn(count) {
t.Log("checkFieldExistsInDocsStep: check function failed")
t.Fail()
return
}
t.Log("check successful")
}

func getDocCountWithField(ctx context.Context, c *elasticsearch.TypedClient, dataStreamName, fieldName string) (int64, error) {
req := &count.Request{
Query: &types.Query{
Exists: &types.ExistsQuery{
Field: fieldName,
},
},
}
resp, err := c.Core.Count().
Index(dataStreamName).
Request(req).
Do(ctx)
if err != nil {
return 0, fmt.Errorf("cannot retrieve doc count: %w", err)
}
return resp.Count, nil
}
1 change: 1 addition & 0 deletions integrationservertest/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ require (
github.com/blang/semver/v4 v4.0.0 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/elastic/elastic-transport-go/v8 v8.6.0 // indirect
github.com/elastic/elasticsearch-serverless-go v0.1.1-20231031 // indirect
github.com/go-logr/logr v1.4.2 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/go-openapi/analysis v0.21.2 // indirect
Expand Down
2 changes: 2 additions & 0 deletions integrationservertest/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ github.com/elastic/cloud-sdk-go v1.23.0 h1:IH41W2E+GIlHjuPtkNPbpYoACAJ8ffk3QiyDj
github.com/elastic/cloud-sdk-go v1.23.0/go.mod h1:k0ZebhZKX22l6Ysl5Zbpc8VLF54hfwDtHppEEEVUJ04=
github.com/elastic/elastic-transport-go/v8 v8.6.0 h1:Y2S/FBjx1LlCv5m6pWAF2kDJAHoSjSRSJCApolgfthA=
github.com/elastic/elastic-transport-go/v8 v8.6.0/go.mod h1:YLHer5cj0csTzNFXoNQ8qhtGY1GTvSqPnKWKaqQE3Hk=
github.com/elastic/elasticsearch-serverless-go v0.1.1-20231031 h1:6zBFaT5Klj/0zHi/35wfTUhy1lgvf/tC9mStYRW9dYA=
github.com/elastic/elasticsearch-serverless-go v0.1.1-20231031/go.mod h1:6DwhqJIaaVnN8AOAfAG0BRG3EsxoQ4ai/NAOxK20sJE=
github.com/elastic/go-elasticsearch/v8 v8.16.0 h1:f7bR+iBz8GTAVhwyFO3hm4ixsz2eMaEy0QroYnXV3jE=
github.com/elastic/go-elasticsearch/v8 v8.16.0/go.mod h1:lGMlgKIbYoRvay3xWBeKahAiJOgmFDsjZC39nmO3H64=
github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc=
Expand Down
4 changes: 4 additions & 0 deletions integrationservertest/internal/ech/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ func (v Version) String() string {
return fmt.Sprintf("%d.%d.%d%s", v.Major, v.Minor, v.Patch, suffix)
}

func (v Version) MajorMinorPatch() string {
return fmt.Sprintf("%d.%d.%d", v.Major, v.Minor, v.Patch)
}

func (v Version) MajorMinor() string {
return fmt.Sprintf("%d.%d", v.Major, v.Minor)
}
Expand Down
4 changes: 4 additions & 0 deletions integrationservertest/internal/elasticsearch/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ func formatDurationElasticsearch(d time.Duration) string {
return fmt.Sprintf("%dnanos", d)
}

func (c *Client) TypedClient() *elasticsearch.TypedClient {
return c.es
}

// CreateAPIKey creates an API Key, and returns it in the base64-encoded form
// that agents should provide.
//
Expand Down
Loading