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

Translate awsentity processor last when translating pipelines #1428

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

phugay
Copy link

@phugay phugay commented Nov 15, 2024

Description of the issue

  • This PR is to make sure awsentity processor to be last in pipelines to consume necessary data from the previously added processors.

Description of changes

  • For example, currently there could be a discrepancy where the ASG is in the appsignals metrics/logs, but it may not appear in compass. With putting awsentity processor last, we can leverage the tags retrieved from the resourcedetection processor to populate ASG for entity when appsignals is enabled.

License

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tests

  • Modified a couple of existing unit tests
  • Verified nothing changes on EC2 as this change is specific to k8s.
  • Verified on EKS cluster by deploying my image, putting comparison logs below:

With default image on cluster:

before adding my image:

service:
    extensions:
        - awsproxy/application_signals
        - agenthealth/traces
        - agenthealth/logs
        - entitystore
        - server
    pipelines:
        metrics/application_signals:
            exporters:
                - awsemf/application_signals
            processors:
                - metricstransform/application_signals
                - awsentity/service/application_signals
                - resourcedetection
                - awsapplicationsignals
            receivers:
                - otlp/application_signals
        metrics/containerinsights:
            exporters:
                - awsemf/containerinsights
            processors:
                - metricstransform/containerinsights
                - gpuattributes/containerinsights
                - batch/containerinsights
            receivers:
                - awscontainerinsightreceiver
        traces/application_signals:
            exporters:
                - awsxray/application_signals
            processors:
                - resourcedetection
                - awsapplicationsignals
            receivers:
                - otlp/application_signals

With my image on cluster:

service:
    extensions:
        - awsproxy/application_signals
        - agenthealth/traces
        - agenthealth/logs
        - entitystore
        - server
    pipelines:
        metrics/application_signals:
            exporters:
                - awsemf/application_signals
            processors:
                - metricstransform/application_signals
                - resourcedetection
                - awsapplicationsignals
                - awsentity/service/application_signals
            receivers:
                - otlp/application_signals
        metrics/containerinsights:
            exporters:
                - awsemf/containerinsights
            processors:
                - metricstransform/containerinsights
                - gpuattributes/containerinsights
                - batch/containerinsights
            receivers:
                - awscontainerinsightreceiver
        traces/application_signals:
            exporters:
                - awsxray/application_signals
            processors:
                - resourcedetection
                - awsapplicationsignals
            receivers:
                - otlp/application_signals

Can successfully see the order change as expected.

Requirements

Before commit the code, please do the following steps.

  1. Run make fmt and make fmt-sh
  2. Run make lint

@phugay phugay requested a review from a team as a code owner November 15, 2024 16:28
@lisguo
Copy link
Contributor

lisguo commented Nov 15, 2024

Unit tests might fail...we want to update the yamls too -

Copy link
Contributor

@Paramadon Paramadon left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants