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

[Ingest Manager] Prevent reporting ecs version twice #21616

Merged
merged 4 commits into from
Oct 19, 2020

Conversation

michalpristas
Copy link
Contributor

@michalpristas michalpristas commented Oct 7, 2020

What does this PR do?

Fixes: #20666

Due to this, we get two ecs.versions in a resulting event.

e.g

...

        "ecs": {
            "version": "1.6.0"
        },
        "ecs.version": "1.5.0",
        "event": {
            "dataset": "elastic.agent.metricbeat"
        },
...

Funny thing is also that it might differ in version as in the example above. But this may be just due to different builds of agent and beat locally.

Why is it important?

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/ingest-management (Team:Ingest Management)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Oct 7, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 7, 2020

💚 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 #21616 updated]

  • Start Time: 2020-10-19T06:43:18.982+0000

  • Duration: 37 min 58 sec

Test stats 🧪

Test Results
Failed 0
Passed 1394
Skipped 4
Total 1398

@ph
Copy link
Contributor

ph commented Oct 7, 2020

@michalpristas I am concerned.. why the version could mismatch? I believe we are using the ecs go library to generate the version is that correct? With that I would expect that beats and Agent to be on the same version?

@@ -113,7 +113,6 @@ func (b *Monitor) EnrichArgs(process, pipelineID string, args []string, isSideca
logFile = fmt.Sprintf("%s-json.log", logFile)
appendix = append(appendix,
"-E", "logging.json=true",
"-E", "logging.ecs=true",
Copy link
Contributor

Choose a reason for hiding this comment

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

I am confused? If this is removed then what is added the ecs.version to the events?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

confused as well i'm still looking, but i think this is not ideal place to fix, need to play with it a bit more

Copy link
Contributor Author

@michalpristas michalpristas Oct 7, 2020

Choose a reason for hiding this comment

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

One ECS is probably comming from processing pipeline (look for WithECS) and additional one we were adding when logging.

This is why i see 2 versions 1.5.0 and 1.6.0. 1.6.0 is coming from publishing pipeline and version is set in libbeat.
1.5.0 is coming from logp as it is hardcoded there.

And as i dont want to alter publishing pipeline and libbeat code, this seems like a good place for a fix after all.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's a problem with logp, that version should be centralized. WDYT @urso

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that explains why. Agent is already adding the ecs fields in the log, then filebeat is adding it again.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does enabling the ecs flags do more than just adding the version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i compared event with ecs enabled and without ecs and they differed in version, host and agent stayed there

Copy link
Contributor

@blakerouse blakerouse left a comment

Choose a reason for hiding this comment

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

Approving because removing that setting is actually correct in this case. I think the ecs version issue in logo should be handled separately.

@michalpristas michalpristas merged commit ee7d329 into elastic:master Oct 19, 2020
michalpristas added a commit to michalpristas/beats that referenced this pull request Oct 19, 2020
[Ingest Manager] Prevent reporting ecs version twice (elastic#21616)
v1v added a commit to v1v/beats that referenced this pull request Oct 19, 2020
* upstream/master: (23 commits)
  [Ingest Manager] Prevent reporting ecs version twice (elastic#21616)
  [CI] Use google storage to keep artifacts (elastic#21910)
  Update docs.asciidoc (elastic#21849)
  Kubernetes leaderelection improvements (elastic#21896)
  Apply name changes to elastic agent docs (elastic#21549)
  Add 7.7.1 relnotes to 7.8 docs (elastic#21937) (elastic#21941)
  [libbeat] Fix potential deadlock in the disk queue + add more unit tests (elastic#21930)
  Refactor docker watcher to fix flaky test and other small issues (elastic#21851)
  [CI] Add stage name in the step (elastic#21887)
  [docs] Remove extra word in autodiscover docs (elastic#21871)
  [CI] lint stage doesn't produce test reports (elastic#21888)
  Add tests of reader of filestream input (elastic#21814)
  [Ingest Manager] Use local temp instead of system one (elastic#21883)
  chore: delegate variant pushes to the right method (elastic#21861)
  [CI] kind setup fails sometimes (elastic#21857)
  Fix panic on add_docker_metadata close (elastic#21882)
  Add tests for fileProspector in filestream input (elastic#21712)
  [Filebeat][okta] Fix okta pagination (elastic#21797)
  Add cloud.account.id into add_cloud_metadata for gcp (elastic#21776)
  Fix syslog RFC 5424 parsing in CheckPoint module (elastic#21854)
  ...
v1v added a commit to v1v/beats that referenced this pull request Oct 19, 2020
* upstream/master: (23 commits)
  [Ingest Manager] Prevent reporting ecs version twice (elastic#21616)
  [CI] Use google storage to keep artifacts (elastic#21910)
  Update docs.asciidoc (elastic#21849)
  Kubernetes leaderelection improvements (elastic#21896)
  Apply name changes to elastic agent docs (elastic#21549)
  Add 7.7.1 relnotes to 7.8 docs (elastic#21937) (elastic#21941)
  [libbeat] Fix potential deadlock in the disk queue + add more unit tests (elastic#21930)
  Refactor docker watcher to fix flaky test and other small issues (elastic#21851)
  [CI] Add stage name in the step (elastic#21887)
  [docs] Remove extra word in autodiscover docs (elastic#21871)
  [CI] lint stage doesn't produce test reports (elastic#21888)
  Add tests of reader of filestream input (elastic#21814)
  [Ingest Manager] Use local temp instead of system one (elastic#21883)
  chore: delegate variant pushes to the right method (elastic#21861)
  [CI] kind setup fails sometimes (elastic#21857)
  Fix panic on add_docker_metadata close (elastic#21882)
  Add tests for fileProspector in filestream input (elastic#21712)
  [Filebeat][okta] Fix okta pagination (elastic#21797)
  Add cloud.account.id into add_cloud_metadata for gcp (elastic#21776)
  Fix syslog RFC 5424 parsing in CheckPoint module (elastic#21854)
  ...
v1v added a commit to v1v/beats that referenced this pull request Oct 19, 2020
…laky-test-analyser

* upstream/master: (22 commits)
  [Ingest Manager] Prevent reporting ecs version twice (elastic#21616)
  [CI] Use google storage to keep artifacts (elastic#21910)
  Update docs.asciidoc (elastic#21849)
  Kubernetes leaderelection improvements (elastic#21896)
  Apply name changes to elastic agent docs (elastic#21549)
  Add 7.7.1 relnotes to 7.8 docs (elastic#21937) (elastic#21941)
  [libbeat] Fix potential deadlock in the disk queue + add more unit tests (elastic#21930)
  Refactor docker watcher to fix flaky test and other small issues (elastic#21851)
  [CI] Add stage name in the step (elastic#21887)
  [docs] Remove extra word in autodiscover docs (elastic#21871)
  [CI] lint stage doesn't produce test reports (elastic#21888)
  Add tests of reader of filestream input (elastic#21814)
  [Ingest Manager] Use local temp instead of system one (elastic#21883)
  chore: delegate variant pushes to the right method (elastic#21861)
  [CI] kind setup fails sometimes (elastic#21857)
  Fix panic on add_docker_metadata close (elastic#21882)
  Add tests for fileProspector in filestream input (elastic#21712)
  [Filebeat][okta] Fix okta pagination (elastic#21797)
  Add cloud.account.id into add_cloud_metadata for gcp (elastic#21776)
  Fix syslog RFC 5424 parsing in CheckPoint module (elastic#21854)
  ...
v1v added a commit to v1v/beats that referenced this pull request Oct 20, 2020
…-store-in-another-location-too

* upstream/master:
  [Ingest Manager] Prevent reporting ecs version twice (elastic#21616)
  [CI] Use google storage to keep artifacts (elastic#21910)
  Update docs.asciidoc (elastic#21849)
  Kubernetes leaderelection improvements (elastic#21896)
  Apply name changes to elastic agent docs (elastic#21549)
  Add 7.7.1 relnotes to 7.8 docs (elastic#21937) (elastic#21941)
  [libbeat] Fix potential deadlock in the disk queue + add more unit tests (elastic#21930)
  Refactor docker watcher to fix flaky test and other small issues (elastic#21851)
  [CI] Add stage name in the step (elastic#21887)
  [docs] Remove extra word in autodiscover docs (elastic#21871)
  [CI] lint stage doesn't produce test reports (elastic#21888)
  Add tests of reader of filestream input (elastic#21814)
  [Ingest Manager] Use local temp instead of system one (elastic#21883)
michalpristas added a commit that referenced this pull request Oct 21, 2020
[Ingest Manager] Prevent reporting ecs version twice (#21616)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Elastic Agent] Logs should only contain ecs.version once
4 participants