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

[Elastic Agent ] Support of processors functionality with hints autodiscovery #2959

Closed
2 tasks done
gizas opened this issue Jun 29, 2023 · 16 comments
Closed
2 tasks done
Assignees
Labels
enhancement New feature or request Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team

Comments

@gizas
Copy link
Contributor

gizas commented Jun 29, 2023

Describe the enhancement:
Currently Elastic Agent Autodiscovery with Hints supports specific list of annotations.
The full list of both required and optional annotations supported can be found in
https://www.elastic.co/guide/en/fleet/current/hints-annotations-autodiscovery.html#_required_hints

This feature will introduce an additional annotation: co.elastic.hints/processor and the goal is the users with this annotation to specify the addition of a new processor

Describe a specific use case for the enhancement or feature:

Users will define eg.
The following annotations

co.elastic.hints/processors.1.add_fields.target: "project"
co.elastic.hints/processors.1.add_fields.fields.name: "myproject"

Elastic agent will emmit the following configuration:

processors:
  - add_fields:
      target: project
      fields:
        name: myproject

What is the definition of done?

Related Issue

Tasks

Preview Give feedback
  1. 8.10-candidate backport-skip
    gizas
  2. backport-8.10 v8.10.0
    gizas kilfoyle
@gizas gizas added enhancement New feature or request Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team labels Jun 29, 2023
@gizas gizas changed the title Support of processors in hints autodiscovery for Elastic Agent [Elastic Agent ] Support of processors functionality with hints autodiscovery Jul 3, 2023
@ChrsMark
Copy link
Member

ChrsMark commented Jul 3, 2023

Hey @gizas thanks for filing this one!

Because of #735 we cannot support a generic approach like the following:

  1. If a package's template already defines 1 or more processors then extend the processors list with what is provided from the hints. (this hits Support list expansion based on dynamic variables #735)
  2. If a package's template does not define processors at then only use any processors defined by the hints.

The reason for 1) failing is that sth like the following is not a valid yaml at all:

processors:
  - add_fields:
      fields:
        custom2: foo
      target: baz
 ${local_dynamic.procs}

Thus we don;t have a way to define a placeholder to be populated by the hints' emmited dictionaries.

I see 3 options here:

A) Find a way to support all the usecases by solving #735.
B) For packages that already define a processors key do not support any additional processors. Only for packages that do not define a processors section have the template generation job to add the following:

processors:
 ${local_dynamic.procs}

C) Based on the assumptions that only Filebeat supported the equivalent only support the processors hint for a container_logs template like the following:

inputs:
  - name: filestream-generic
    id: hints-container-logs-${kubernetes.hints.container_id}
    type: filestream
    use_output: default
    streams:
      - condition: ${kubernetes.hints.generic_logs.container_logs.enabled} == true
        data_stream:
          dataset: kubernetes.container_logs
          type: logs
        exclude_files: [ ]
        exclude_lines: [ ]
        parsers:
          - container:
              format: auto
              stream: ${kubernetes.hints.generic_logs.container_logs.stream|'all'}
        paths:
          - /var/log/containers/*${kubernetes.hints.container_id}.log
        prospector:
          scanner:
            symlinks: true
        tags: [ ]
    data_stream.namespace: default
    processors:
      ${kubernetes.hints.generic_logs.container_logs.procs}

Let me know what you think, and we can try to validate options B or C to verify that we don't miss anything.

@gizas
Copy link
Contributor Author

gizas commented Jul 7, 2023

After our sync we have verified that processors map that is being used when we update hints mapping (see https://github.com/elastic/elastic-agent/blob/main/internal/pkg/composable/providers/kubernetes/pod.go#L218) can read from configuration file the additional processors and append additional processor blocks.

I have created the branch to track the work
https://github.com/elastic/elastic-agent/tree/processorhints_enhancement

@gsantoro if you want quickly to verify that processors defined in our config are also added to the final input after hint generation see below steps:

  1. go build your elastic agent
  2. Change the elastic-agent.yml file with the following (see below we add a new processor)
providers.kubernetes:
  node: "pas-control-plane"
  kube_config: /Users/andreasgkizas/.kube/config
  #Uncomment to enable hints' support
  hints.enabled: true

inputs:
  # Collecting system metrics
  - name: filestream-generic
    id: hints-container-logs-${kubernetes.hints.container_id}
    type: filestream
    use_output: default
    processors:
      - add_fields:
          fields:
            name: myproject
            id: '574734885120952459'
    streams:
      - condition: ${kubernetes.hints.generic_logs.container_logs.enabled} == true
        data_stream:
          dataset: kubernetes.container_logs
          type: logs
        exclude_files: [ ]
        exclude_lines: [ ]
        parsers:
          - container:
              format: auto
              stream: ${kubernetes.hints.generic_logs.container_logs.stream|'all'}
        paths:
          - /var/log/containers/*${kubernetes.hints.container_id}.log
        prospector:
          scanner:
            symlinks: true
    data_stream.namespace: default

Run inspect command to see rendered output:

./elastic-agent inspect -v --variables --variables-wait 2s

Rendered output:

./elastic-agent inspect -v --variables --variables-wait 2s
agent:
  logging:
    to_stderr: true
inputs:
- data_stream.namespace: default
  id: hints-container-logs-34bad0cd3deeffb8b2029b9615f6777567192619c4ad52a614cbd84c84cb895c-kubernetes-dc8a5011-15ac-4579-81a8-31a93fe9b721.nginx
  name: filestream-generic
  original_id: hints-container-logs-34bad0cd3deeffb8b2029b9615f6777567192619c4ad52a614cbd84c84cb895c
  processors:
  - add_fields:
      fields:
        id: 34bad0cd3deeffb8b2029b9615f6777567192619c4ad52a614cbd84c84cb895c
        image:
          name: nginx
        runtime: containerd
      target: container
  - add_fields:
      fields:
        container:
          name: nginx
        deployment:
          name: nginx
        labels:
          app: nginx
          pod-template-hash: 654557c679
        namespace: default
        namespace_labels:
          kubernetes_io/metadata_name: default
        namespace_uid: 052c58b8-c5c0-4f4c-bcb2-4bf3a471583f
        node:
          hostname: pas-control-plane
          labels:
            beta_kubernetes_io/arch: arm64
            beta_kubernetes_io/os: linux
            kubernetes_io/arch: arm64
            kubernetes_io/hostname: pas-control-plane
            kubernetes_io/os: linux
            node-role_kubernetes_io/control-plane: ""
            node_kubernetes_io/exclude-from-external-load-balancers: ""
          name: pas-control-plane
          uid: 0511824c-88f3-4b38-a0a5-406ed2eda282
        pod:
          ip: 10.244.0.5
          name: nginx-654557c679-5pg8g
          uid: dc8a5011-15ac-4579-81a8-31a93fe9b721
        replicaset:
          name: nginx-654557c679
      target: kubernetes
  - add_fields:
      fields:
        cluster:
          name: kind-pas
          url: https://127.0.0.1:57202
      target: orchestrator
  - add_fields:
      fields:
        id: "574734885120952459"
        name: myproject
  streams:
  - data_stream:
      dataset: kubernetes.container_logs
[output trucnated ...]

So long strory short is that on branch above I just retrieve any processors that are part of config and I append them in the existing processor map.

@ChrsMark
Copy link
Member

ChrsMark commented Jul 7, 2023

@gizas we just need to make sure we document that processors from hints will only be added on integration/input/package level and not on data_stream level.

As a first iteration this is fine but let's make sure document it and even file an issue for Elastic Agent team to evaluate if we can tune the providers' interface so as to emit processors also on data_stream's level (this happens at

as already mentioned)

@gizas
Copy link
Contributor Author

gizas commented Jul 7, 2023

Sure sure! Lets first evaluate and test this and we will do!!! Thanks man once more @ChrsMark !

@gizas
Copy link
Contributor Author

gizas commented Jul 19, 2023

I have pushed latest changes in :
https://github.com/elastic/elastic-agent/tree/processorhints_enhancement

I build the above elastic agent image and load it to elastic-standalone by following https://github.com/elastic/elastic-agent#testing-elastic-agent-on-kubernetes

Please have in mind that this is a draft.

So the goal is to read the processors annotations from pod and append those in the processor map that is going to be emitted.

In more details:
Agent config:

providers.kubernetes:
      node: ${NODE_NAME}
      scope: node
      hints:
        enabled: true
        default_container_logs: false 

And I mount the /etc/elastic-agent/inputs.d/container_logs.yml template of https://github.com/elastic/elastic-agent/blob/main/deploy/kubernetes/elastic-agent-standalone/templates.d/container_logs.yml

nginx.yml

apiVersion: apps/v1
kind: Deployment
metadata:
  labels:
    app: nginx
  name: nginx
  namespace: default
spec:
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
      annotations:
        co.elastic.hints/package: "container_logs"
        co.elastic.hints/processors.1.add_fields.target: "project"
        co.elastic.hints/processors.1.add_fields.fields.name: "myproject"  
    spec:
      containers:
      - image: nginx
        name: nginx

Nginx is installed with id:

k describe pod nginx-65c56f67cb-2svvr | grep -i id
    Container ID:   containerd://52efcae02df01b7cdd359914dc3d16eb6b419236c9a4fd0f9f6b88e84c67574c

So I see the following flow:

  1. hitns autodiscovery starts https://github.com/elastic/elastic-agent/blob/processorhints_enhancement/internal/pkg/composable/providers/kubernetes/pod.go#L213
  2. For specific nginx pod this new part of the code works and retrieves the processor added: https://github.com/elastic/elastic-agent/blob/processorhints_enhancement/internal/pkg/composable/providers/kubernetes/pod.go#L553
    We return the processorMapping
  3. Then line 484: https://github.com/elastic/elastic-agent/blob/processorhints_enhancement/internal/pkg/composable/providers/kubernetes/pod.go#L484 handles correctly the processors list

This is what is in processorlist
Screenshot 2023-07-19 at 3 45 30 PM

This is what is in final processor map https://github.com/elastic/elastic-agent/blob/processorhints_enhancement/internal/pkg/composable/providers/kubernetes/pod.go#L493

Screenshot 2023-07-19 at 3 46 10 PM

But when I resume the debugger and expect that the final configuration will be emitted in agent, it does not !

See output:

elastic-agent inspect -v --variables --variables-wait 2s
I0719 12:52:22.681261      91 leaderelection.go:248] attempting to acquire leader lease kube-system/elastic-agent-cluster-leader...
I0719 12:52:22.688726      91 leaderelection.go:258] successfully acquired lease kube-system/elastic-agent-cluster-leader
agent:
  logging:
    to_stderr: true
inputs:
- data_stream.namespace: default
  id: unique-system-metrics-input
  streams:
  - data_stream.dataset: system.cpu
    metricsets:
    - cpu
  - data_stream.dataset: system.memory
    metricsets:
    - memory
  - data_stream.dataset: system.network
    metricsets:
    - network
  - data_stream.dataset: system.filesystem
    metricsets:
    - filesystem
  type: system/metrics
  use_output: default
outputs:
  default:
    hosts: http://elasticsearch:9200
    password: changeme
    type: elasticsearch
    username: elastic

@ChrsMark
Copy link
Member

@gizas I'm not sure if the inspect command honors the templates that are in the inputs.d directory.
Actually I see that the inputs are not being populated at all.

Did you try to put the template within the agent.yml main configuration file instead and try the inspect command?
I usually follow this workflow to avoid the extra overhead of the inputs.d directory. See the testing notes from #2386 to get an idea.

@gizas
Copy link
Contributor Author

gizas commented Jul 19, 2023

Hmm sth seems broken as still not works, and inspect does not show anything
but when I change the image to the ones that we have not build all populate

And yes the inspect honours the templates under inputs.d, but I also checked manually

@ChrsMark
Copy link
Member

If you comment out the part that extends the processors list at e4d1c95#diff-ed933ad9cf09eb9ca797c5c89b5e8991624cbe5301d88f996d9bd947f4e3ea16R450-R455 for example, what do you see?

Also I would try to add a dummy processor body there as an initial step and see it it works.
We need to ensure that we can extend this list without an issue. I guess it should be ok but we need to verify this.

Also maybe adding some unit tests will help here to debug it easier.

If none of these help, I can find some time to checkout the branch and try it on my end.

@gizas
Copy link
Contributor Author

gizas commented Jul 19, 2023

It worked !!!!!

- data_stream.namespace: default
  id: hints-filestream-container-logs-52efcae02df01b7cdd359914dc3d16eb6b419236c9a4fd0f9f6b88e84c67574c-kubernetes-5d63b8c2-bbff-4e04-9bd8-7337555aa861.nginx
  name: hints-filestream-container-logs
  original_id: hints-filestream-container-logs-52efcae02df01b7cdd359914dc3d16eb6b419236c9a4fd0f9f6b88e84c67574c
  processors:
  - add_fields:
      fields:
        id: 52efcae02df01b7cdd359914dc3d16eb6b419236c9a4fd0f9f6b88e84c67574c
        image:
          name: nginx
        runtime: containerd
      target: container
  [output truncated .....]
  - add_fields:
      fields:
        name: myproject
      target: project
  streams:
  - data_stream:
      dataset: kubernetes.container_logs
      type: logs
    parsers:
    - container:
        format: auto
        stream: all
    paths:
    - /var/log/containers/*52efcae02df01b7cdd359914dc3d16eb6b419236c9a4fd0f9f6b88e84c67574c.log

You can not believe what it was!!!!
I added in the agent config allow_older_versions: true (my stack is 8.9.0-SNAPSHOT) and the agent is 8.10.0!!! I should had looked in agent logs as it was failing to connect to Elasticsearch, thus was failing to proceed.

@gsantoro
Copy link
Contributor

hey @gizas ,
I was able to replicate the behaviour but I had to use

./elastic-agent inspect -c /etc/elastic-agent/agent.yml -v --variables --variables-wait 2s

when running the command from the elastic-agent pod. Notice the param -c with the configuration to use mounted from the ConfigMap.

@gsantoro
Copy link
Contributor

gsantoro commented Jul 25, 2023

hey @gizas and @ChrsMark I am not able to replicate the outcome, I was able to run the command and generate a policy but that policy doesn't match the output that you created. I was able to build the code from your feature branch, run the debugger, but the output elastic-agent config doesn't look the same as yours.

@gsantoro
Copy link
Contributor

gsantoro commented Jul 25, 2023

hello @gizas and @ChrsMark I finally managed to get the correct policy printed by elastic-agent and the data to be correctly printed into ES. I'm not sure if it was me not being aware of how hints work but it took me an entire day to replicate this issue. This is what I have done:

  1. Started elastic-agent built from feature branch processorhints_enhancement with the following agent.yml config
outputs:
  default:
    type: elasticsearch
    hosts:
      - >-
        ${ES_HOST}
    protocol: https
    ssl.verification_mode: "${ES_SSL_VERIFICATION_MODE}"
    allow_older_versions: ${ES_ALLOW_OLDER_VERSIONS}
    username: ${ES_USERNAME}
    password: ${ES_PASSWORD}
agent:
  monitoring:
    enabled: true
    use_output: default
    logs: true
    metrics: true
providers.kubernetes:
  node: ${NODE_NAME}
  scope: node
  #Uncomment to enable hints' support
  hints.enabled: true
inputs:
  # Collecting system metrics
  - name: filestream-generic
    id: hints-container-logs-${kubernetes.hints.container_id}
    type: filestream
    use_output: default
    processors:
      - add_fields:
          fields:
            name: myproject
            id: '574734885120952459'
    streams:
      - condition: ${kubernetes.hints.kubernetes.container_logs.enabled} == true
        data_stream:
          dataset: kubernetes.container_logs
          type: logs
        exclude_files: [ ]
        exclude_lines: [ ]
        parsers:
          - container:
              format: auto
              stream: ${kubernetes.hints.kubernetes.container_logs.stream|'all'}
        paths:
          - /var/log/containers/*${kubernetes.hints.container_id}.log
        prospector:
          scanner:
            symlinks: true
    data_stream.namespace: default

To notice from above that:

  • I'm using kubernetes.hints.kubernetes.container_logs instead of kubernetes.hints.generic_logs.container_logs
  • I've never used kube_config: /Users/andreasgkizas/.kube/config. I don't know what it is but we don't need it

I used a pod with the following annotations

annotations:
  co.elastic.hints/package: "kubernetes"
  co.elastic.hints/data_streams: "container_logs"
  co.elastic.hints/processors.1.add_fields.target: "project"
  co.elastic.hints/processors.1.add_fields.fields.name: "myproject"

To notice that I "guessed" those hints from looking at the redis docs at #2386.

  1. I didn't use the config
default_container_logs: false 

I don't know what it is but we don't need it.

  1. I used the following command to get the generated policy into my laptop
kubectl exec -n kube-system -it elastic-agent-standalone-f422w -- ./elastic-agent inspect -c /etc/elastic-agent/agent.yml --variables --variables-wait 2s > ./policy.yaml

Clearly you need to adapt the previous command to the elastic-agent pod that you are running.

  1. The output policy has both the fixed field set in the original policy and the extra field from the hints
inputs:
- data_stream.namespace: default
  id: hints-container-logs-ddcae4cf518fc3645461f6ae903da145713376bc73c128d6a05ded1dbb0421f0-kubernetes-fdf67bc3-394c-495c-b87e-02b11b90898f.nginx
  name: filestream-generic
  original_id: hints-container-logs-ddcae4cf518fc3645461f6ae903da145713376bc73c128d6a05ded1dbb0421f0
  processors:
  - add_fields:
      fields:
        id: ddcae4cf518fc3645461f6ae903da145713376bc73c128d6a05ded1dbb0421f0
        image:
          name: nginx:1.23.1-alpine
        runtime: containerd
      target: container
  - add_fields:
      fields:
        name: myproject
      target: project
  - add_fields:
      fields:
        id: "574734885120952459"
        name: myproject
  streams:
  - data_stream:
      dataset: kubernetes.container_logs
      type: logs
    exclude_files: []
    exclude_lines: []
    parsers:
    - container:
        format: auto
        stream: all
    paths:
    - /var/log/containers/*ddcae4cf518fc3645461f6ae903da145713376bc73c128d6a05ded1dbb0421f0.log
    prospector:
      scanner:
        symlinks: true
  type: filestream
  use_output: default
  1. the data correctly contains that extra field

I hope that's all we wanted to test.

@ChrsMark
Copy link
Member

ChrsMark commented Jul 25, 2023

I think that default_container_logs: false is needed for this test.

Also the

annotations:
  co.elastic.hints/package: "kubernetes"
  co.elastic.hints/data_streams: "container_logs"
  co.elastic.hints/processors.1.add_fields.target: "project"
  co.elastic.hints/processors.1.add_fields.fields.name: "myproject"

looks weird to me. There is no kubernetes package-template as far as I can I see in the templates dir. It works because you defined the hints as kubernetes.hints.kubernetes.container_logs.enabled.
Also see how the template looks like at https://github.com/elastic/elastic-agent/blob/main/deploy/kubernetes/elastic-agent-standalone/templates.d/container_logs.yml#L7. IN any case that is the template that will be used by the end users so you need to test with this.

Not sure what else you might have missed here:).

I could reproduce the feature locally even without running from inside the Pod. Note that the inspect command can be executed from your local machine outside the kind cluster if you point to the proper kube_config.

Here is what I use:

providers:
  kubernetes:
    kube_config: /home/chrismark/.kube/config
    node: "kind-worker"
    hints.enabled: true
    hints.default_container_logs: false

inputs:
  - name: hints-filestream-container-logs
    id: hints-filestream-container-logs-${kubernetes.hints.container_id}
    type: filestream
    use_output: default
    streams:
      - condition: ${kubernetes.hints.container_logs.enabled} == true
        data_stream:
          dataset: kubernetes.container_logs
          type: logs
        parsers:
          - container:
              format: auto
              stream: ${kubernetes.hints.container_logs.stream|'all'}
        paths:
          - /var/log/containers/*${kubernetes.hints.container_id}.log
        prospector:
          scanner:
            symlinks: true
    data_stream.namespace: default

I run ./elastic-agent inspect -v --variables --variables-wait 2s

to get:

agent:
  logging:
    to_stderr: true
inputs:
- data_stream.namespace: default
  id: hints-filestream-container-logs-d8af5ef2c60a2b5bdb7f2bd239d6994a5015f71bf70cb5584f6a3b86ed0f82ca-kubernetes-e46a6433-dd51-496c-a987-45d78a63b50f.nginx
  name: hints-filestream-container-logs
  original_id: hints-filestream-container-logs-d8af5ef2c60a2b5bdb7f2bd239d6994a5015f71bf70cb5584f6a3b86ed0f82ca
  processors:
  - add_fields:
      fields:
        id: d8af5ef2c60a2b5bdb7f2bd239d6994a5015f71bf70cb5584f6a3b86ed0f82ca
        image:
          name: nginx
        runtime: containerd
      target: container
  - add_fields:
      fields:
        container:
          name: nginx
        deployment:
          name: nginx
        labels:
          app: nginx
          pod-template-hash: 678f56544b
        namespace: default
        namespace_labels:
          kubernetes_io/metadata_name: default
        namespace_uid: ba30ee51-a9ac-4044-96bc-c110183aeb2d
        node:
          hostname: kind-worker
          labels:
            beta_kubernetes_io/arch: amd64
            beta_kubernetes_io/os: linux
            kubernetes_io/arch: amd64
            kubernetes_io/hostname: kind-worker
            kubernetes_io/os: linux
          name: kind-worker
          uid: 2f643ef8-be15-45b1-9972-ceb7c91c81f6
        pod:
          ip: 10.244.1.124
          name: nginx-678f56544b-wd892
          uid: e46a6433-dd51-496c-a987-45d78a63b50f
        replicaset:
          name: nginx-678f56544b
      target: kubernetes
  - add_fields:
      fields:
        cluster:
          name: kind-kind
          url: https://127.0.0.1:37345
      target: orchestrator
  - add_fields:
      fields:
        name: myproject
      target: project
  - rename:
      fail_on_error: "false"
      fields:
        "0":
          from: a.g
        "1":
          to: e.d
  streams:
  - data_stream:
      dataset: kubernetes.container_logs
      type: logs
    parsers:
    - container:
        format: auto
        stream: all
    paths:
    - /var/log/containers/*d8af5ef2c60a2b5bdb7f2bd239d6994a5015f71bf70cb5584f6a3b86ed0f82ca.log
    prospector:
      scanner:
        symlinks: true
  type: filestream
  use_output: default
outputs:
  default:
    api_key: example-key
    hosts:
    - 127.0.0.1:9200
    type: elasticsearch
providers:
  kubernetes:
    hints:
      default_container_logs: false
      enabled: true
    kube_config: /home/chrismark/.kube/config
    node: kind-worker

@gsantoro
Copy link
Contributor

thanks for the feedback @ChrsMark, it works as expected now. A couple of points:

  1. default_container_logs: false I still have no idea what this is used for. If mentioned in a doc somewhere, I would have appreciated having a link in the PR.
  2. About kubernetes.hints.container_logs this prefix was never mentioned in this ticket before you mentioned it now. I believe that both you and @gizas used kubernetes.hints.generic_logs.container_logs instead and then mentioned co.elastic.hints/package: "container_logs" in the hints of the container. Those two values didn't match and there wasn't a generic_logs.container_logs template either. Also, I had no idea what those values had to match with. I got to my working result totally randomly, but I believe if that wasn't meant to work it should have failed somehow. As I said, that could have been my fault since I had no prior knowledge of hints but not having this information here or a link to a doc didn't really help.
  3. I found it very confusing that the docs at Fix log collection with hints when missing container ports or annotations #2386 mention a package redis and two datastreams info, log letting me guess that those are references to integrations and data streams, while the hints in this PR use the same hints package to refer to a template container_logs instead. Is this intentional? Again, maybe I'm missing some documentation that I would have appreciated to have linked here.

I'm just glad that is it all solved now and I appreciate the help, but given the complexity of this setup and the fact that I had no prior knowledge about hints and how docs are sometimes not enough or out of date, for me, it's very important having a self-contained PR with all the info to replicate the issue.

I hope that you understand

@ChrsMark
Copy link
Member

@gsantoro all the questions are sane and valuable feedback. However keep in mind that the feature is still in Beta, and there is active development happening here so lot's of things might be missing in terms of docs etc. The purpose of #3063 is to identify such issues and make the feature as stable as possible.

So first we need to answer the question of what is the purpose of your testing as part of this issue: Is it to get familiar with the codebase/feature and actively contribute to it or just to perform a QA testing? A QA testing without codebase knowledge at this point might does not make sense since there is still active development and docs etc might be missing. If the goal is to get familiar with the codebase then the process of trial and error you described sounds normal and is the learning curve related to this advanced feature. Answering the questions specifically inline:

  1. default_container_logs: false I still have no idea what this is used for. If mentioned in a doc somewhere, I would have appreciated having a link in the PR.

I think we indeed miss the respective docs part from https://www.elastic.co/guide/en/fleet/8.9/hints-annotations-autodiscovery.html. It should have been added after #2386 but priorities changed so we missed that (my bad mostly :) ). Feel free to file an issue for this or a PR directly. cc-ing @gizas to ensure this becomes part of the #3063.

  1. About kubernetes.hints.container_logs this prefix was never mentioned in this ticket before you mentioned it now. I believe that both you and @gizas used kubernetes.hints.generic_logs.container_logs instead and then mentioned co.elastic.hints/package: "container_logs" in the hints of the container. Those two values didn't match and there wasn't a generic_logs.container_logs template either. Also, I had no idea what those values had to match with. I got to my working result totally randomly, but I believe if that wasn't meant to work it should have failed somehow. As I said, that could have been my fault since I had no prior knowledge of hints but not having this information here or a link to a doc didn't really help.

I don't get it. The template that @gizas points out in testing notes of the respective PR is https://github.com/elastic/elastic-agent/blob/main/deploy/kubernetes/elastic-agent-standalone/templates.d/container_logs.yml. And this one contains the kubernetes.hints.container_logs, no? What do I miss here?

  1. I found it very confusing that the docs at Fix log collection with hints when missing container ports or annotations #2386 mention a package redis and two datastreams info, log letting me guess that those are references to integrations and data streams, while the hints in this PR use the same hints package to refer to a template container_logs instead. Is this intentional? Again, maybe I'm missing some documentation that I would have appreciated to have linked here.

I don't see how #2386 is related to this PR and what docs you are referring to. If you mean the "How to test this manually" section I think the examples are accurate with one called Multiple containers case and one called Generic logs collection for all non annotated containers which actually test the code changes.

I'm just glad that is it all solved now and I appreciate the help, but given the complexity of this setup and the fact that I had no prior knowledge about hints and how docs are sometimes not enough or out of date, for me, it's very important having a self-contained PR with all the info to replicate the issue.

I hope that you understand

In general if you think that the testing notes of #3107 should be more accurate/correct please suggest to @gizas directly what is missing and how it would be better.

@gsantoro
Copy link
Contributor

thanks for the clarification:

  1. About

So first we need to answer the question of what is the purpose of your testing as part of this issue

I have no idea. It was pointed out above at

@gsantoro if you want quickly to verify that processors defined in our config are also added to the final input after hint generation see below steps:

  1. Ah yes, the PR pointed contains the right config, anything else in this issue points at generic_logs.container_logs until you posted the new config. If you try to search for kubernetes.hints.container_logs you were the first to point that out. I'm just complaining because I hope we can do a better job in the future.

  2. About Fix log collection with hints when missing container ports or annotations #2386 you mentioned at [Elastic Agent ] Support of processors functionality with hints autodiscovery #2959 (comment). Why is it relevant, I am not sure. Even after reading your comment I am not sure what's the difference between the multiple containers case and the generic logs use case.

  3. about

In general if you think that the testing notes of #3107 should be more accurate/correct please suggest to @gizas directly what is missing and how it would be better.

I think they are accurate. It's a pity that I wasted 2 days looking at the wrong instructions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Team:Cloudnative-Monitoring Label for the Cloud Native Monitoring team
Projects
None yet
Development

No branches or pull requests

3 participants