Logging code cleanup related to Nomad auto-discovery

[andrewkroh]

What does this PR do?

This does some minor clean up of the logging code and docs related to Nomad. I removed some statements that were providing duplicate information and switched a few over to using the structured logger to improve log readability.

A log message about enabled not being parsed was always being logged despite no actual error.

The doc examples were trying to start a Filebeat input with type: nomad.

Fixed some typos in code comments.

Why is it important?

This make debugging a little more enjoyable 😆 .

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.

Related issues

• Closes Nomad auto-discover uses Debugf incorrectly #26456

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Build Cause: andrewkroh commented: run tests

• Start Time: 2021-06-28T20:14:20.169+0000

• Duration: 97 min 40 sec

• Commit: 7352205

Test stats 🧪

Test	Results
Failed	0
Passed	29481
Skipped	3839
Total	33320

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	29481
Skipped	3839
Total	33320

[andrewkroh]

run tests

[wjordan]

I have a semi-related note about this path template (sorry if slightly off-topic)- when testing out this feature I noticed that the task.name data was only being published on the meta.nomad field of the event, not the nomad field used in this example. Here's the relevant code:

beats/x-pack/libbeat/autodiscover/providers/nomad/nomad.go

Lines 197 to 210 in 076e0a6

	event := bus.Event{
	"provider": p.uuid,
	"id": fmt.Sprintf("%s-%s", obj.ID, task["name"]),
	flag: true,
	"host": nodeName,
	"nomad": allocMeta,
	"meta": common.MapStr{
	"nomad": common.MapStrUnion(allocMeta, common.MapStr{
	"task": task,
	}),
	},
	}
	p.publish(event)

In my tests, ${data.nomad.task.name} gave an invalid template reference error, but ${data.meta.nomad.task.name} resolved properly. Does this sound correct and if so, should all the documentation examples be updated to reflect this working usage? Or does a separate PR to fix the existing examples make more sense?

[andrewkroh]

When testing I noted that there was a lot of duplicate information in the autodiscover events that get published by that code. But I wasn't sure what the expected behavior was. And now that you bring it up, I see that there are docs for what should be in the events at https://www.elastic.co/guide/en/beats/filebeat/current/configuration-autodiscover.html#_nomad. So based on those docs I think I could change this and fix the missing nomad.task fields. Can you open a separate issue please?

Here's a sample document taken from the updated debug logs I added.

{
  "autodiscover.event": {
    "provider": "d4bc811c-9039-44a5-be01-fd60b08c1a6a",
    "meta": {
      "nomad": {
        "namespace": "default",
        "allocation": {
          "name": "hello-world.servers[0]",
          "id": "64782844-aedc-4acb-b72d-30dfe7f7939d",
          "status": "running"
        },
        "datacenter": [
          "va"
        ],
        "task": {
          "name": "hello-world"
        },
        "region": "global",
        "job": {
          "name": "hello-world",
          "type": "service"
        }
      }
    },
    "host": "compute01",
    "start": true,
    "id": "64782844-aedc-4acb-b72d-30dfe7f7939d-hello-world",
    "nomad": {
      "namespace": "default",
      "allocation": {
        "name": "hello-world.servers[0]",
        "id": "64782844-aedc-4acb-b72d-30dfe7f7939d",
        "status": "running"
      },
      "datacenter": [
        "va"
      ],
      "region": "global",
      "job": {
        "name": "hello-world",
        "type": "service"
      }
    },
    "config": []
  }
}

[wjordan]

Can you open a separate issue please?

Opened #26538.

But I wasn't sure what the expected behavior was.

It turns out there was some discussion around the need for this duplicate information in the original PR- see #14954 (comment).

[andrewkroh]

Great find. So based on that comment it sounds like we need to keep both meta and nomad. But we can make sure all the documented keys are present under the nomad.

[jsoriano]

Good catch!

I think only the nomad fields need to be available for config templates. The meta fields are used for enrichment, but shouldn't be necessarily available for templates. We probably messed something up in the original PR, I remember going back and forth on this 😬

We also have to double-check that hints based configs have the same fields available.

[andrewkroh]

Note to self, two other things to fix:

• Docs show secretID as config value but it's secret_id.

• Docs are missing info about ACL requirements. Seems like filebeat needs an ACL token with these capabilities.

namespace "*" {
  policy = "read"
}
node {
  policy = "read"
}
agent {
  policy = "read"
}

[elasticmachine]

Pinging @elastic/integrations (Team:Integrations)

[andrewkroh]

run tests

[jsoriano]

Thanks! Glad to see some activity around nomad 🙂