search

open-telemetry / opentelemetry-collector-contrib

Contrib repository for the OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
2.94k stars 2.29k forks source link

AWS Cloud Map usage at Load Balancing has a max limit up to 100 instances #29771

Open andretong opened 9 months ago

andretong commented 9 months ago

Component(s)

exporter/loadbalancing

Describe the issue you're reporting

Given the usage of the AWS Cloud Map SDK to discover the Collector's instances implemented in the PR 27588

There is a limitation regarding the amount of instances which by default is 100. SDK reference

The idea for this enhancement is to be able to override this via configuration to a max value more according to your use case, for instance, if you expect to have up to 200 instances, then we can set 200 as MaxResults for this service discovery request.

github-actions[bot] commented 9 months ago

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

crobert-1 commented 9 months ago

It might be good to add more context to your issue description here. From the PR you've linked this is an enhancement request to add another configuration option (or maybe use the max value of 100 every time?), and then to handle the pagination of returned hosts. Is that right? If you could make all of this clear in the description it would be really helpful so we don't have to refer back to the PR to learn about the context and request here.

andretong commented 9 months ago

It might be good to add more context to your issue description here. From the PR you've linked this is an enhancement request to add another configuration option (or maybe use the max value of 100 every time?), and then to handle the pagination of returned hosts. Is that right? If you could make all of this clear in the description it would be really helpful so we don't have to refer back to the PR to learn about the context and request here.

Thanks for your comment @crobert-1 I've updated the description with the overall idea. As initially checked into the SDK, seems that this MaxResults configuration can be set with a max total instances expected in your environment. This would help us to avoid adding a pagination logic in this exported

github-actions[bot] commented 7 months ago

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

github-actions[bot] commented 5 months ago

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

crobert-1 commented 5 months ago

Resolved by https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/27588

ptodev commented 4 months ago

Resolved by #27588

@crobert-1 @andretong Is this issue really resolved by that PR? The PR also adds this comment to the docs:

TODO: Feature request https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/29771 aims to cover the pagination for this scenario

crobert-1 commented 4 months ago

Thanks for bringing this up @ptodev, I agree it looks like I closed this incorrectly. I'll reopen and defer to others 👍

github-actions[bot] commented 2 months ago

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

jpkrohling commented 2 months ago

@andretong, are you addressing this one?

github-actions[bot] commented 2 weeks ago

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

jpkrohling commented 2 weeks ago

@andretong, I'm deprecating this resolver and removing it from the next version of the load balancer. I'm not ready to maintain this component alone, and calls to complete it should be addressed.

andretong commented 1 day ago

@jpkrohling Sorry, I missed this message, I can start to work on having this corner case covered, but in the meantime can this resolver be kept as maintained? On the other hand, as far as I checked, this hasn't been a reported issue.