add end to end test for declarative config for aws #2047
Conversation
| ComponentLoader.forClassLoader(ResourceComponentProviderTest.class.getClassLoader()) | ||
| .load(ComponentProvider.class); | ||
| assertThat(providers).extracting(ComponentProvider::getName).containsExactly("aws"); | ||
| assertThat(providers).extracting(ComponentProvider::getName).contains("aws"); |
There was a problem hiding this comment.
I was curious about the need for changing containsExactly to contains so i tried to put this back to containsExactly to see what would happen, and noticed that in the test failure it lists console 3 times, plus an additional service:
Expecting actual:
["aws", "console", "console", "console", "service"]
to contain exactly (and in same order):
["aws"]
but some elements were not expected:
["console", "console", "console", "service"]
I changed the test code to get some insights into what was actually loaded to see. Looks like this is because declarative configuration is automatically adding additional console exporters for metrics and logs plus the service resource detector.
Expecting actual:
[io.opentelemetry.contrib.aws.resource.internal.AwsResourceDetector,
io.opentelemetry.exporter.logging.internal.ConsoleMetricExporterComponentProvider,
io.opentelemetry.exporter.logging.internal.ConsoleSpanExporterComponentProvider,
io.opentelemetry.exporter.logging.internal.ConsoleLogRecordExporterComponentProvider,
io.opentelemetry.sdk.extension.incubator.fileconfig.ServiceResourceDetector]
This surprised me a bit because I thought if the metric and log exporters weren't explicitly defined, they wouldn't be configured. Is my understanding off here?
There was a problem hiding this comment.
ComponentProvider is only about what providers are available to be loaded if they appear in the yaml
This test seems redundant when the e2e test is there - I'll delete it
jaydeluca
left a comment
jaydeluca
left a comment
There was a problem hiding this comment.
I'll add the other cloud providers, too, if this is a good pattern.
This seems like a reasonable approach to me 👍
zeitlinger
force-pushed
the
cloud-provider-end-to-end-test
branch
from
06919f9 to
b169e27
Compare
github-actions
bot
requested review from
KarstenSchnitter,
cyrille-leclerc,
kenfinnigan,
srprash,
trask and
wangzlei
|
@jaydeluca added the same style of tests except for GCP and appserver, because they only work on a particular environment that can't easily be mocked |
KarstenSchnitter
left a comment
KarstenSchnitter
left a comment
There was a problem hiding this comment.
Thanks for providing the change. I reviewed the cloud-foundry resource changes. Looks good to me.
|
@jaydeluca can you check again? |
jaydeluca
left a comment
jaydeluca
left a comment
There was a problem hiding this comment.
👍 since these endToEnd tests are all very similar we could probably create a general AbstractResourceProviderTest or utility class and just pass the attributes
there is no "test-common" project in contrib - and I'm not sure this use case justifies it |
4 participants
See #2014 (comment)
I'll add the other cloud providers, too, if this is a good pattern.