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

Add aws-otel-collector #50

Merged
merged 2 commits into from
Sep 10, 2024
Merged

Conversation

koooosh
Copy link
Contributor

@koooosh koooosh commented Jul 24, 2024

Issue number: #52

Closes #52

Description of changes:

This change adds the aws-otel-collector package to the core-kit to be used by our downstream customers. Specifically, it includes the following:

  • aws-otel-collector.spec which provides non-FIPS and FIPS versions of the collector binary
  • Cargo.toml for the new package
  • aws-otel-collector.service which is the systemd service for the collector. This is similar to the default service with the following changes:
    • Removing the EnvironmentFile field and providing the config path directly in ExecStart. This way if downstream customers want to provide their own config, they just have to add a drop-in unit updating the ExecStart field.
    • Changing Restart from on-failure to always just for safety
    • Reducing the RestartSec from 60s to 5s
    • Removing the User and Group fields since they are unnecessary
    • Changing network.target to network-online.target configured.target in relevant settings
  • aws-otel-collector.yaml which is a minimal config needed for the collector to run
  • aws-otel-collector-tmpfiles.conf to install above config in /etc rather than the default /opt
  • 0001-change-logger-and-extraconfig-file-paths.patch to change the log and extraconfig file paths from /opt to /var/log and /etc, respectively
  • Updates to the bottlerocket-core-kit/Cargo.toml and root Cargo.toml and Cargo.lock to add the aws-otel-collector to the kit

Testing done:

Launched an aws-dev variant instance with the aws-otel-collector package added and verified the following:

  • Collector service is running successfully:
bash-5.2# systemctl status aws-otel-collector
● aws-otel-collector.service - AWS OTEL collector
     Loaded: loaded (/x86_64-bottlerocket-linux-gnu/sys-root/usr/lib/systemd/system/aws-otel-collector.service; enabled; preset: enabled)
     Active: active (running) since Wed 2024-07-24 07:24:58 UTC; 14h ago
   Main PID: 1346 (aws-otel-collec)
      Tasks: 9 (limit: 18599)
     Memory: 134.7M
        CPU: 13.451s
     CGroup: /system.slice/aws-otel-collector.service
             └─1346 /usr/bin/aws-otel-collector --config /etc/aws-otel-collector-default-config.yaml
  • Config file is installed correctly in /etc:
bash-5.2# ls /etc/ | grep otel
aws-otel-collector-default-config.yaml
  • Log file is installed in correct location in /var/log:
bash-5.2# ls /var/log/aws/
aws-otel-collector.log

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

@koooosh koooosh changed the title Add otel collector Add aws-otel-collector Jul 24, 2024
@koooosh koooosh self-assigned this Jul 24, 2024
@koooosh koooosh marked this pull request as ready for review July 24, 2024 21:43
@koooosh koooosh requested a review from bcressey July 24, 2024 21:44
Version: 0.40.0
Release: 1%{?dist}
Summary: An AWS supported service to send telemetry data
License: Apache-2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

This license statement seems incorrect. You can run askalono crawl . in a directory with all the unpacked sources to get its verdict (askalono). But you are probably looking at an aggregation of all the licenses in THIRD-PARTY-LICENSES.txt.

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 see. I based this off the Linux build.spec file offered by the collector repo: https://github.com/aws-observability/aws-otel-collector/blob/main/tools/packaging/linux/build.spec#L5

When I run askalono crawl . | grep License: | sort | uniq on the unpacked collector directory, I get the following excluding errors:

License: Apache-2.0 (license header)
License: Apache-2.0 (original text)
License: BSD-2-Clause (original text)
License: BSD-2-Clause-Views (original text)
License: BSD-3-Clause (original text)
License: CC-BY-SA-4.0 (original text)
License: ISC (original text)
License: MIT (original text)
License: MPL-2.0 (original text)

Given the license header is listed as Apache-2.0, is this correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Went through all the licenses in the THIRD-PARTY file and aggregated this:

License: Apache-2.0 AND BSD-2-Clause AND BSD-3-Clause AND MIT AND MPL-2.0

packages/aws-otel-collector/aws-otel-collector.spec Outdated Show resolved Hide resolved
packages/aws-otel-collector/aws-otel-collector.spec Outdated Show resolved Hide resolved
packages/aws-otel-collector/aws-otel-collector.spec Outdated Show resolved Hide resolved
packages/aws-otel-collector/aws-otel-collector.service Outdated Show resolved Hide resolved
Comment on lines 1 to 2
extensions:
health_check:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not entirely sure it makes sense to ship a config at all here, but if we do then it should be a bare minimum - just the most basic one possible with all optional functionality turned off.

The awsxray, awsemf, http stuff I would cut.

Downstream variants can pretty easily specify a different config file via systemd unit drop-in that overrides the ExecStart command to point to their custom config or rendered template.

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 believe the collector requires some sort of config which specifies receivers, processors, and exporters; otherwise it will fail. I tried creating a patch to make the config optional but it causes the collector to fail with invalid configuration errors with ResolverSettings, which is an import from the general OTEL collector pkg.

I've created a minimalist config based on possible core (non-aws specific) components which the collector still functions with here: https://gist.github.com/koooosh/8cddd42a91657552af4709b5d1157bef

Let me know if you notice anything off-putting in that.

packages/aws-otel-collector/aws-otel-collector.service Outdated Show resolved Hide resolved
@koooosh koooosh force-pushed the add-otel-collector branch from a57b0aa to 3aa8a8d Compare August 8, 2024 19:52
@koooosh
Copy link
Contributor Author

koooosh commented Aug 8, 2024

Force push addresses @bcressey comments:

  • Updates to aws-otel-collector.spec, namely correcting License statement
  • Changed aws-otel-collector-default-config.yaml to just aws-otel-collector.yaml and made the config as minimal as possible with localhost endpoints
  • Update settings in aws-otel-collector.service
  • Updating patch flags in amazon-ssm-agent and socat for consistency

@koooosh koooosh requested review from arnaldo2792 and bcressey August 8, 2024 21:02
@koooosh koooosh force-pushed the add-otel-collector branch from 3aa8a8d to 15bb5d7 Compare August 15, 2024 10:58
@koooosh
Copy link
Contributor Author

koooosh commented Aug 15, 2024

Force push addresses @arnaldo2792 comments:

  • Removing socat and amazon-ssm-agent fix commits
  • Updating patch with extraconfig path change

@koooosh koooosh requested a review from arnaldo2792 August 15, 2024 12:43
Copy link
Member

@larvacea larvacea left a comment

Choose a reason for hiding this comment

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

I looked through this, and especially looked through the remaining conversations on the pull request, and it looks good to me.

Signed-off-by: Kush Upadhyay <kushupad@amazon.com>
Signed-off-by: Kush Upadhyay <kushupad@amazon.com>
@koooosh
Copy link
Contributor Author

koooosh commented Sep 5, 2024

Force push updates the following:

  • Updates extraconfig file path in patch to: "/etc/aws-otel-collector/extracfg.txt"
  • Changes Restart setting for collector service to always
  • Rearranges source ordering

@koooosh koooosh requested a review from arnaldo2792 September 5, 2024 16:57
@koooosh koooosh requested a review from bcressey September 6, 2024 16:42
@koooosh koooosh merged commit d9aa79f into bottlerocket-os:develop Sep 10, 2024
2 checks passed
@koooosh koooosh deleted the add-otel-collector branch October 3, 2024 01:01
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.

Add aws-otel-collector package to core-kit
4 participants