Improve reliability of tests (#643)

* Run tests on Windows in Github Actions

* core sha update

* format code

* fix ci yaml

* rebase

* lint

* Try without win+py3.6 fix

* Try without win+py3.6 fix

* Improve test reliability

Update some tests to use more deterministic methods of testing in memory
spans. This helps the core repo pass tests after adding Windows to CI
matrix.

.github/workflows/test.yml

@@ -6,7 +6,7 @@ on:
     - 'release/*'
   pull_request:
 env:
-  CORE_REPO_SHA: 9dc17e33bc083e38c1fd55b8ed6787caba8f54fe
+  CORE_REPO_SHA: c49ad57bfe35cfc69bfa863d74058ca9bec55fc3
 
 jobs:
   build:

exporter/opentelemetry-exporter-datadog/tests/test_datadog_exporter.py

@@ -14,12 +14,14 @@
 
 import itertools
 import logging
+import sys
 import time
 import unittest
 from unittest import mock
 
 from ddtrace.internal.writer import AgentWriter
 from flaky import flaky
+from pytest import mark
 
 from opentelemetry import trace as trace_api
 from opentelemetry.context import Context
@@ -469,6 +471,9 @@ class TestDatadogSpanExporter(unittest.TestCase):
         self.assertEqual(len(datadog_spans), 128)
         tracer_provider.shutdown()
 
+    @mark.skipif(
+        sys.platform == "win32", reason="unreliable test on windows",
+    )
     def test_span_processor_scheduled_delay(self):
         """Test that spans are exported each schedule_delay_millis"""
         delay = 300

instrumentation/opentelemetry-instrumentation-botocore/tests/test_botocore_instrumentation.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 import io
 import json
+import sys
 import zipfile
 from unittest.mock import Mock, patch
 
@@ -30,6 +31,7 @@ from moto import (  # pylint: disable=import-error
     mock_sts,
     mock_xray,
 )
+from pytest import mark
 
 from opentelemetry import trace as trace_api
 from opentelemetry.context import attach, detach, set_value
@@ -128,12 +130,13 @@ class TestBotocoreInstrumentor(TestBase):
         s3.list_buckets()
         s3.list_buckets()
 
-        spans = self.memory_exporter.get_finished_spans()
+        spans = self.get_finished_spans()
         assert spans
-        span = spans[0]
         self.assertEqual(len(spans), 2)
+
+        buckets_span = spans.by_attr("aws.operation", "ListBuckets")
         self.assertSpanHasAttributes(
-            span,
+            buckets_span,
             {
                 "aws.operation": "ListBuckets",
                 "aws.region": "us-west-2",
@@ -144,14 +147,13 @@ class TestBotocoreInstrumentor(TestBase):
         )
 
         # testing for span error
-        self.memory_exporter.get_finished_spans()
         with self.assertRaises(ParamValidationError):
             s3.list_objects(bucket="mybucket")
-        spans = self.memory_exporter.get_finished_spans()
+        spans = self.get_finished_spans()
         assert spans
-        span = spans[2]
+        objects_span = spans.by_attr("aws.operation", "ListObjects")
         self.assertSpanHasAttributes(
-            span,
+            objects_span,
             {
                 "aws.operation": "ListObjects",
                 "aws.region": "us-west-2",
@@ -159,7 +161,7 @@ class TestBotocoreInstrumentor(TestBase):
             },
         )
         self.assertIs(
-            span.status.status_code, trace_api.StatusCode.ERROR,
+            objects_span.status.status_code, trace_api.StatusCode.ERROR,
         )
 
     # Comment test for issue 1088
@@ -172,10 +174,11 @@ class TestBotocoreInstrumentor(TestBase):
         s3.put_object(**params)
         s3.get_object(Bucket="mybucket", Key="foo")
 
-        spans = self.memory_exporter.get_finished_spans()
+        spans = self.get_finished_spans()
         assert spans
         self.assertEqual(len(spans), 3)
-        create_span = spans[0]
+
+        create_span = spans.by_attr("aws.operation", "CreateBucket")
         self.assertSpanHasAttributes(
             create_span,
             {
@@ -186,7 +189,8 @@ class TestBotocoreInstrumentor(TestBase):
                 SpanAttributes.HTTP_STATUS_CODE: 200,
             },
         )
-        put_span = spans[1]
+
+        put_span = spans.by_attr("aws.operation", "PutObject")
         self.assertSpanHasAttributes(
             put_span,
             {
@@ -197,8 +201,10 @@ class TestBotocoreInstrumentor(TestBase):
                 SpanAttributes.HTTP_STATUS_CODE: 200,
             },
         )
-        self.assertTrue("params.Body" not in spans[1].attributes.keys())
+        self.assertTrue("params.Body" not in put_span.attributes.keys())
-        get_span = spans[2]
+
+        get_span = spans.by_attr("aws.operation", "GetObject")
+
         self.assertSpanHasAttributes(
             get_span,
             {
@@ -359,6 +365,10 @@ class TestBotocoreInstrumentor(TestBase):
             Path="/my-path/",
         )["Role"]["Arn"]
 
+    @mark.skipif(
+        sys.platform == "win32",
+        reason="requires docker and Github CI Windows does not have docker installed by default",
+    )
     @mock_lambda
     def test_lambda_invoke_propagation(self):
 

instrumentation/opentelemetry-instrumentation-elasticsearch/tests/test_elasticsearch.py

@@ -60,19 +60,13 @@ class TestElasticsearchIntegration(TestBase):
         with self.disable_logging():
             ElasticsearchInstrumentor().uninstrument()
 
-    def get_ordered_finished_spans(self):
-        return sorted(
-            self.memory_exporter.get_finished_spans(),
-            key=lambda s: s.start_time,
-        )
-
     def test_instrumentor(self, request_mock):
         request_mock.return_value = (1, {}, {})
 
         es = Elasticsearch()
         es.index(index="sw", doc_type="people", id=1, body={"name": "adam"})
 
-        spans_list = self.get_ordered_finished_spans()
+        spans_list = self.get_finished_spans()
         self.assertEqual(len(spans_list), 1)
         span = spans_list[0]
 
@@ -87,7 +81,7 @@ class TestElasticsearchIntegration(TestBase):
 
         es.index(index="sw", doc_type="people", id=1, body={"name": "adam"})
 
-        spans_list = self.get_ordered_finished_spans()
+        spans_list = self.get_finished_spans()
         self.assertEqual(len(spans_list), 1)
 
     def test_span_not_recording(self, request_mock):
@@ -127,7 +121,7 @@ class TestElasticsearchIntegration(TestBase):
         es = Elasticsearch()
         es.index(index="sw", doc_type="people", id=1, body={"name": "adam"})
 
-        spans_list = self.get_ordered_finished_spans()
+        spans_list = self.get_finished_spans()
         self.assertEqual(len(spans_list), 1)
         span = spans_list[0]
         self.assertTrue(span.name.startswith(prefix))
@@ -141,7 +135,7 @@ class TestElasticsearchIntegration(TestBase):
         es = Elasticsearch()
         es.get(index="test-index", doc_type="tweet", id=1)
 
-        spans = self.get_ordered_finished_spans()
+        spans = self.get_finished_spans()
 
         self.assertEqual(1, len(spans))
         self.assertEqual("False", spans[0].attributes["elasticsearch.found"])
@@ -169,7 +163,7 @@ class TestElasticsearchIntegration(TestBase):
         except Exception:  # pylint: disable=broad-except
             pass
 
-        spans = self.get_ordered_finished_spans()
+        spans = self.get_finished_spans()
         self.assertEqual(1, len(spans))
         span = spans[0]
         self.assertFalse(span.status.is_ok)
@@ -186,13 +180,13 @@ class TestElasticsearchIntegration(TestBase):
                 index="sw", doc_type="people", id=1, body={"name": "adam"}
             )
 
-        spans = self.get_ordered_finished_spans()
+        spans = self.get_finished_spans()
         self.assertEqual(len(spans), 2)
 
-        self.assertEqual(spans[0].name, "parent")
+        parent = spans.by_name("parent")
-        self.assertEqual(spans[1].name, "Elasticsearch/sw/people/1")
+        child = spans.by_name("Elasticsearch/sw/people/1")
-        self.assertIsNotNone(spans[1].parent)
+        self.assertIsNotNone(child.parent)
-        self.assertEqual(spans[1].parent.span_id, spans[0].context.span_id)
+        self.assertEqual(child.parent.span_id, parent.context.span_id)
 
     def test_multithread(self, request_mock):
         request_mock.return_value = (1, {}, {})
@@ -222,16 +216,15 @@ class TestElasticsearchIntegration(TestBase):
         t1.join()
         t2.join()
 
-        spans = self.get_ordered_finished_spans()
+        spans = self.get_finished_spans()
         self.assertEqual(3, len(spans))
-        s1, s2, s3 = spans
 
-        self.assertEqual(s1.name, "parent")
+        s1 = spans.by_name("parent")
+        s2 = spans.by_name("Elasticsearch/test-index/tweet/1")
+        s3 = spans.by_name("Elasticsearch/test-index/tweet/2")
 
-        self.assertEqual(s2.name, "Elasticsearch/test-index/tweet/1")
         self.assertIsNotNone(s2.parent)
         self.assertEqual(s2.parent.span_id, s1.context.span_id)
-        self.assertEqual(s3.name, "Elasticsearch/test-index/tweet/2")
         self.assertIsNone(s3.parent)
 
     def test_dsl_search(self, request_mock):
@@ -242,7 +235,7 @@ class TestElasticsearchIntegration(TestBase):
             "term", author="testing"
         )
         search.execute()
-        spans = self.get_ordered_finished_spans()
+        spans = self.get_finished_spans()
         span = spans[0]
         self.assertEqual(1, len(spans))
         self.assertEqual(span.name, "Elasticsearch/test-index/_search")
@@ -270,10 +263,11 @@ class TestElasticsearchIntegration(TestBase):
         client = Elasticsearch()
         Article.init(using=client)
 
-        spans = self.get_ordered_finished_spans()
+        spans = self.get_finished_spans()
         self.assertEqual(2, len(spans))
-        span1, span2 = spans
+        span1 = spans.by_attr(key="elasticsearch.method", value="HEAD")
-        self.assertEqual(span1.name, "Elasticsearch/test-index")
+        span2 = spans.by_attr(key="elasticsearch.method", value="PUT")
+
         self.assertEqual(
             span1.attributes,
             {
@@ -283,7 +277,6 @@ class TestElasticsearchIntegration(TestBase):
             },
         )
 
-        self.assertEqual(span2.name, "Elasticsearch/test-index")
         attributes = {
             SpanAttributes.DB_SYSTEM: "elasticsearch",
             "elasticsearch.url": "/test-index",
@@ -306,7 +299,7 @@ class TestElasticsearchIntegration(TestBase):
         )
         res = article.save(using=client)
         self.assertTrue(res)
-        spans = self.get_ordered_finished_spans()
+        spans = self.get_finished_spans()
         self.assertEqual(1, len(spans))
         span = spans[0]
         self.assertEqual(span.name, helpers.dsl_index_span_name)

instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py

@@ -185,15 +185,19 @@ class TestTornadoInstrumentation(TornadoTest):
     def _test_async_handler(self, url, handler_name):
         response = self.fetch(url)
         self.assertEqual(response.code, 201)
-        spans = self.memory_exporter.get_finished_spans()
+        spans = self.get_finished_spans()
         self.assertEqual(len(spans), 5)
 
-        sub2, sub1, sub_wrapper, server, client = self.sorted_spans(spans)
+        client = spans.by_name("GET")
+        server = spans.by_name(handler_name + ".get")
+        sub_wrapper = spans.by_name("sub-task-wrapper")
 
+        sub2 = spans.by_name("sub-task-2")
         self.assertEqual(sub2.name, "sub-task-2")
         self.assertEqual(sub2.parent, sub_wrapper.context)
         self.assertEqual(sub2.context.trace_id, client.context.trace_id)
 
+        sub1 = spans.by_name("sub-task-1")
         self.assertEqual(sub1.name, "sub-task-1")
         self.assertEqual(sub1.parent, sub_wrapper.context)
         self.assertEqual(sub1.context.trace_id, client.context.trace_id)
@@ -238,9 +242,11 @@ class TestTornadoInstrumentation(TornadoTest):
         response = self.fetch("/error")
         self.assertEqual(response.code, 500)
 
-        spans = self.sorted_spans(self.memory_exporter.get_finished_spans())
+        spans = self.get_finished_spans()
         self.assertEqual(len(spans), 2)
-        server, client = spans
+
+        client = spans.by_name("GET")
+        server = spans.by_name("BadHandler.get")
 
         self.assertEqual(server.name, "BadHandler.get")
         self.assertEqual(server.kind, SpanKind.SERVER)

tox.ini

@@ -122,7 +122,8 @@ envlist =
     py3{6,7,8,9}-test-instrumentation-grpc
 
     ; opentelemetry-instrumentation-sqlalchemy
-    py3{6,7,8,9}-test-instrumentation-sqlalchemy{11,14}
+    py3{6,7}-test-instrumentation-sqlalchemy{11}
+    py3{6,7,8,9}-test-instrumentation-sqlalchemy{14}
     pypy3-test-instrumentation-sqlalchemy{11,14}
 
     ; opentelemetry-instrumentation-redis