Skip to content

Support data stream type == traces#258

Merged
ycombinator merged 1 commit intoelastic:masterfrom
ycombinator:support-traces
Feb 22, 2021
Merged

Support data stream type == traces#258
ycombinator merged 1 commit intoelastic:masterfrom
ycombinator:support-traces

Conversation

@ycombinator
Copy link
Copy Markdown
Contributor

@ycombinator ycombinator commented Feb 19, 2021 •
edited
Loading

Looking in all three active branches of the package-storage repo, I have confirmed that the apm package is the only one that is using type: traces in one of its data streams. I looked at this data stream and it defines ingest pipelines.

This PR updates elastic-package to support type: traces for the above use case. Specifically:

  • it updates the check for valid data streams to include data streams of type: traces, and
  • it allows running pipeline tests on data streams of type: traces

Resolves #223.

@ycombinator ycombinator changed the title Accounting for data stream type == traces Support data stream type == traces Feb 19, 2021
@ycombinator ycombinator requested review from jalvz and mtojek February 19, 2021 16:22
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Feb 19, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #258 opened

  • Start Time: 2021-02-19T16:22:50.415+0000

  • Duration: 17 min 25 sec

  • Commit: 3b84637

Test stats 🧪

Test Results
Failed 0
Passed 292
Skipped 1
Total 293

Trends 🧪

Image of Build Times

Image of Tests

@jalvz
Copy link
Copy Markdown

jalvz commented Feb 19, 2021

thanks @ycombinator . what were the consequences of this missing? i've been testing this for a while and couldn't notice any issue with traces.

@ycombinator
Copy link
Copy Markdown
Contributor Author

ycombinator commented Feb 19, 2021 •
edited
Loading

what were the consequences of this missing? i've been testing this for a while and couldn't notice any issue with traces.

At this point the consequences are only theoretical because the apm package's traces data stream does not define any pipeline tests. If it did and elastic-package test pipeline --data-stream traces was run, it wouldn't actually run those tests because prior to this PR we only ran pipeline tests for data streams of type logs.

So in a practical sense, this PR is more forward looking. If/when the apm package defines pipeline tests for it's data streams' ingest pipelines, particularly ones for the traces data stream, the changes in this PR will allow those tests to run.

@jalvz
Copy link
Copy Markdown

jalvz commented Feb 22, 2021

got it, thanks @ycombinator !

mtojek
mtojek approved these changes Feb 22, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@mtojek mtojek left a comment

Choose a reason for hiding this comment

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

One more comment to what @ycombinator said - to fully use all these features (including automated testing), it would be great to move the "apm" package to the Integrations repository.

@ycombinator ycombinator merged commit f568e5d into elastic:master Feb 22, 2021
@ycombinator ycombinator deleted the support-traces branch February 22, 2021 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@jalvz jalvz jalvz approved these changes

@mtojek mtojek mtojek approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support traces data stream type

4 participants

@ycombinator @elasticmachine @jalvz @mtojek