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 otelcol shortcut to elastic-agent otel #4816

Merged
merged 14 commits into from
Jun 6, 2024
Merged

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented May 24, 2024

Description

Resolve #4661

Provide an otelcol shortcut to elastic-agent otel cmd.

The solution with a shell script seems to work well for mac and linux, for windows I had to create an elastic-otel.ps1 to be executable. @ycombinator @blakerouse it is acceptable for windows? is there a better implementation possible?

Tests

You can build the agent and test that is working. Should we have some automated test for those new shortcuts? and is there something in place to build them?

Copy link
Contributor

mergify bot commented May 24, 2024

This pull request does not have a backport label. Could you fix it @nchaulet? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 8./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@ycombinator ycombinator added the Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team label May 24, 2024
@strawgate
Copy link

strawgate commented May 28, 2024

thoughts on doing otelcol instead of elastic-otel? or elastic-otel-collector?

@ycombinator
Copy link
Contributor

thoughts on doing otelcol instead of elastic-otel? or elastic-otel-collector?

#4661 (comment)


BASEDIR=$(dirname "$0")

exec $BASEDIR/elastic-agent otel $@

Choose a reason for hiding this comment

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

For Darwin and Linux I believe we actually want

exec "$BASEDIR/elastic-agent" otel "$@"

If you put elastic-agent into a folder with a space and then cd to root and try to run it, it will fail due to the space without the path being quoted.

In addition I believe $@ needs to be in quotes:

($@) Expands to the positional parameters, starting from one. When the expansion occurs within double quotes, each parameter expands to a separate word.

./a.sh 2 "3     4" 5
2 3     4 5                  # output for "$@"
2 3 4 5                      # output for $@  -> spaces are lost!

Copy link
Member

Choose a reason for hiding this comment

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

Yes, run shellcheck on this and it should find this problem and any other Bash quirks.

@nchaulet nchaulet changed the title Add elastic-otel shortcut to elastic-agent otel Add otelcol shortcut to elastic-agent otel May 30, 2024
@nchaulet nchaulet marked this pull request as ready for review May 31, 2024 13:48
@nchaulet nchaulet requested a review from a team as a code owner May 31, 2024 13:48
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-control-plane (Team:Elastic-Agent-Control-Plane)

@ycombinator ycombinator requested review from michalpristas and removed request for michel-laterman June 4, 2024 13:37
@@ -811,10 +832,7 @@ specs:
'data/{{.BeatName}}-{{ commit_short }}/elastic-agent':
template: '{{ elastic_beats_dir }}/dev-tools/packaging/templates/darwin/elastic-agent.tmpl'
mode: 0755
'{{.BeatName}}{{.BinaryExt}}':
Copy link
Contributor

Choose a reason for hiding this comment

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

intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I will re-add this

@nchaulet nchaulet dismissed strawgate’s stale review June 6, 2024 12:39

I made the requested changes

Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@nchaulet nchaulet merged commit 6a02d8b into main Jun 6, 2024
14 checks passed
@nchaulet nchaulet deleted the feature-elastic-otel-cmd branch June 6, 2024 17:39
mergify bot pushed a commit that referenced this pull request Jun 25, 2024
(cherry picked from commit 6a02d8b)

# Conflicts:
#	dev-tools/packaging/packages.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v8.14.0 Team:Elastic-Agent-Control-Plane Label for the Agent Control Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[OTel mode] Provide otelcol executable as a shortcut to elastic-agent otel
7 participants