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

ECS sidecar task container observer #5316

Closed
rmfitzpatrick opened this issue Sep 21, 2021 · 4 comments · Fixed by #6894
Closed

ECS sidecar task container observer #5316

rmfitzpatrick opened this issue Sep 21, 2021 · 4 comments · Fixed by #6894
Labels
comp:aws AWS components enhancement New feature or request

Comments

@rmfitzpatrick
Copy link
Contributor

Is your feature request related to a problem? Please describe.
It's not currently possible to use the receiver creator with the ECS observer as it doesn't discover container endpoints. It was an initial aim of the observer to add this functionality over time but it's possible the extension's architecture, cluster-wide observation, and hard dependency on prometheus file-based service discovery may not be conducive to the use case of running the collector as a sidecar to scrape the task metadata endpoint for container discovery in tandem with the receiver creator. The observer currently recommends the ECS resource detector for sidecar deployments but this incompatible with the requested aim.

In comparison, the Docker observer will provide similar functionality to what is being requested in this ticket but with endpoints discovered from the docker daemon.

Describe the solution you'd like
For the short-term feature gap of the ECS observer, it may be ideal for this functionality to be added to the existing ECS observer.

Describe alternatives you've considered
On the other hand this usage pattern could be applied to the multiple container management offerings like containerd, podman, and azure container instances. A more generalized "container observer" that allows configurable target status endpoints/event queues with a unified config may be warranted. The upcoming docker observer could be converted into a "backend" option for this observer.

Additional context
The SignalFx Smart Agent offers an ECS observer with the desired functionality whose contents Splunk would be happy to contribute to whatever path is recommended.

cc: @tigrannajaryan @mstumpfx @emaderer @pingleig

@pingleig
Copy link
Contributor

I already mentioned this issue in (amazon's) slack channel (but seems there is no reply on github) ... I no longer work on otel collector, we need someone else to help on this cc @Aneurysm9 @alolita

TL;DR I think it's better to create a new observer

I feel container endpoints are a bit different than the initial design of ecsobserver extension, which is running a single collector for entire cluster. This is based on what we did in cloudwatch agent, which may not scale for large clusters due to polling API and resource limit of a single collector.

Regardless of having a new observer or a merge with existing ecsobserver, I'd suggest address #3188 first to avoid creating a third ecs metadata client in otel repo.

The way ecsobserver export endpoint (writing to file sd) can be changed to use receiver creator. We mentioned why we choose to use file sd in #3785 (comment). If you are using metadata API, performance should not be a problem because there is less targets. Current code is extensible when it comes to output format, in the first implementation we allow both file sd and list endpoint listener #1920 In current code you can branch out when you get the targets in memory.

b, err := targetsToFileSDYAML(targets, s.cfg.JobLabelName)

btw: @rakyll is working on some prometheus ecs related things such as https://github.com/prometheus-community/ecs_exporter, though I don't have much context about the scope

@rmfitzpatrick
Copy link
Contributor Author

Thanks for the input, @pingleig. I've not yet fully examined the overall extensibility but my initial impression was in line with your thoughts that since it caters to a different use case an additional observer may be beneficial. However, I think the observer component was initially implemented for receiver creator usage and the current ECS observer is the only one that doesn't support endpoint creation. I think I can see how it would be extended by separating discovery and target outputs based on additional config settings without introducing breaking changes.

If the overall observer/receiver creator performance could be improved and the configuration made intuitive for cluster/task deployments and prometheus sd/endpoints it may be the most effective route to add this enhancement (addressing #3188 as you suggest). This could resolve a few issues while improving the capabilities of the collector as a whole.

@alolita
Copy link
Member

alolita commented Sep 30, 2021

@rmfitzpatrick Let's chat more on this requirement.

@alolita alolita added comp:aws AWS components enhancement New feature or request labels Sep 30, 2021
@rmfitzpatrick
Copy link
Contributor Author

After taking #5316 (comment) into consideration and reviewing the existing observer I think a separate task/sidecar observer is the way to go to avoid unnecessary config/deployment reconciliation and opened #6121.

Per a discussion w/ @alolita I do think the existing ECS observer should be updated to allow observer endpoint creation to work with the receiver creator and am happy to try to add this. Will be sure to share a writeup and poc w/ the suggested enhancements in #1395.

tigrannajaryan pushed a commit that referenced this issue Nov 11, 2021
Adding a feature - These changes introduce a new receiver creator-compatible watch observer for ECS tasks.  It differs from the existing ECS observer in that it supports only sidecar deployments and doesn't have a hard dependency on the Prometheus receiver, instead being able to sync with a receiver creator instance by listing [container endpoints](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/957eb18037f000f5740c4044fdf5ffeee4311ebc/extension/observer/endpoints.go#L166).  This approach was landed on after reviewing the existing and ECS observer config and implementation and from following the guidance of the initial observer's author: #5316 (comment).  It does not resolve [the outstanding item](#1395 (comment)) for #1395, as I think observer endpoint creation should be added to the existing observer as well but should be done in unrelated changes.

**Link to tracking Issue:**
#5316

**Testing:**
~No tests as this time since the changes just include the initial types and readme.~ Basic factory test to help ensure receiver creator type compatibility.*

**Documentation:**
Adding a readme with high level plan and config documentation generated by project configschema tool.
rmfitzpatrick pushed a commit to rmfitzpatrick/opentelemetry-collector-contrib that referenced this issue Nov 11, 2021
These changes unify the two ECS task metadata client implementations from the ECS Resource Detection Processor
and the AWS ECS Container Metrics Receiver. These are intended to be completely transparent but there are minor
implementation detail and interface updates around metadata endpoint detection (and its error reporting) and the
client/client provider types from the container metrics receiver. Container stats remain in the metric receiver
since they are not used by the resource detector. Adoption of core's HTTPClientSettings is also included per a
TODO comment in the receiver.

These changes are also intended to assist the implementation of an ECS sidecar watch observer as mentioned in open-telemetry#5316.
rmfitzpatrick pushed a commit to rmfitzpatrick/opentelemetry-collector-contrib that referenced this issue Nov 15, 2021
These changes unify the two ECS task metadata client implementations from the ECS Resource Detection Processor
and the AWS ECS Container Metrics Receiver. These are intended to be completely transparent but there are minor
implementation detail and interface updates around metadata endpoint detection (and its error reporting) and the
client/client provider types from the container metrics receiver. Container stats remain in the metric receiver
since they are not used by the resource detector. Adoption of core's HTTPClientSettings is also included per a
TODO comment in the receiver.

These changes are also intended to assist the implementation of an ECS sidecar watch observer as mentioned in open-telemetry#5316.
rmfitzpatrick pushed a commit to rmfitzpatrick/opentelemetry-collector-contrib that referenced this issue Nov 16, 2021
These changes unify the two ECS task metadata client implementations from the ECS Resource Detection Processor
and the AWS ECS Container Metrics Receiver. These are intended to be completely transparent but there are minor
implementation detail and interface updates around metadata endpoint detection (and its error reporting) and the
client/client provider types from the container metrics receiver. Container stats remain in the metric receiver
since they are not used by the resource detector. Adoption of core's HTTPClientSettings is also included per a
TODO comment in the receiver.

These changes are also intended to assist the implementation of an ECS sidecar watch observer as mentioned in open-telemetry#5316.
tigrannajaryan pushed a commit that referenced this issue Nov 16, 2021
These changes unify the two ECS task metadata client implementations from the ECS Resource Detection Processor
and the AWS ECS Container Metrics Receiver. These are intended to be completely transparent but there are minor
implementation detail and interface updates around metadata endpoint detection (and its error reporting) and the
client/client provider types from the container metrics receiver. Container stats remain in the metric receiver
since they are not used by the resource detector. Adoption of core's HTTPClientSettings is also included per a
TODO comment in the receiver.

These changes are also intended to assist the implementation of an ECS sidecar watch observer as mentioned in #5316.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:aws AWS components enhancement New feature or request
Projects
None yet
3 participants