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

OtlpExporter: Integration test testing https support #1795

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
a1b43ff
Enable support for both http and https
alanwest Feb 5, 2021
323ed14
Logic is hard
alanwest Feb 5, 2021
d5f6cce
Merge branch 'main' into alanwest/otlp-allow-tls
cijothomas Feb 5, 2021
11a8e85
Merge branch 'main' into alanwest/otlp-allow-tls
cijothomas Feb 5, 2021
852e091
https test that works for netstandard2.1 gRPC library
alanwest Feb 5, 2021
9029055
Apply suggestions from code review
alanwest Feb 5, 2021
b161bda
Fix https test for Google gRPC library
alanwest Feb 5, 2021
4e595e9
Merge branch 'alanwest/otlp-allow-tls' of github.com:alanwest/opentel…
alanwest Feb 5, 2021
66e844a
Add some comments
alanwest Feb 6, 2021
a63867a
Merge branch 'main' into alanwest/otlp-allow-tls
alanwest Feb 6, 2021
bc729c1
Update changelog
alanwest Feb 6, 2021
21bed21
Merge branch 'alanwest/otlp-allow-tls' of github.com:alanwest/opentel…
alanwest Feb 6, 2021
008a9c5
Merge branch 'main' into alanwest/otlp-allow-tls
alanwest Feb 11, 2021
e906d8e
Remove cert from repo
alanwest Feb 12, 2021
af7ce26
Create certificate each test run
alanwest Feb 12, 2021
09b1189
Update workflow
alanwest Feb 12, 2021
6107e05
Merge remote-tracking branch 'upstream/main' into alanwest/otlp-allow…
alanwest Mar 6, 2021
fe51556
Merge branch 'main' into alanwest/otlp-allow-tls
cijothomas Mar 10, 2021
443470e
Merge branch 'main' into alanwest/otlp-allow-tls
cijothomas Apr 7, 2021
fccd624
Merge branch 'main' into alanwest/otlp-allow-tls
cijothomas Apr 8, 2021
1db8833
Merge branch 'main' into alanwest/otlp-allow-tls
cijothomas Apr 14, 2021
db3ed09
Merge branch 'main' into alanwest/otlp-allow-tls
cijothomas Oct 7, 2021
9d812d0
Merge branch 'main' into alanwest/otlp-allow-tls
cijothomas Oct 13, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,4 @@ jobs:
steps:
- uses: actions/checkout@v2
- name: Run OTLP Exporter docker-compose.integration
run: docker-compose --file=test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/docker-compose.yml --file=build/docker-compose.${{ matrix.version }}.yml --project-directory=. up --exit-code-from=tests --build
run: docker-compose --file=test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/integration-test/docker-compose.yml --file=build/docker-compose.${{ matrix.version }}.yml --project-directory=. up --exit-code-from=tests --build
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ public void ExportResultIsSuccess()

var exporterOptions = new OtlpExporterOptions
{
Endpoint = new System.Uri($"http://{CollectorEndpoint}"),
// TODO: Should test both http and https
Endpoint = new System.Uri($"https://{CollectorEndpoint}"),
Comment on lines +43 to +44
Copy link
Member Author

Choose a reason for hiding this comment

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

Just to vet out https support, I just changed the test to use https. I plan to follow up and have it test both.

Copy link
Member

Choose a reason for hiding this comment

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

If we use a self-signed cert might need to manually pass validation somehow?

Copy link
Member Author

Choose a reason for hiding this comment

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

If we use a self-signed cert might need to manually pass validation somehow?

The cert gets installed as a trusted cert here, so no need to manually pass validation:

RUN apt-get update && apt-get install ca-certificates
COPY --from=build /repo/test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/otel-collector-config/otel-collector.crt /usr/local/share/ca-certificates/
RUN update-ca-certificates --verbose

I had actually tried this first:

ServicePointManager.ServerCertificateValidationCallback = (sender, certificate, chain, policyErrors) => true;

For some reason this did not work... I've used this kind of thing in test applications before, but for some reason the callback was not getting invoked by the test.

Copy link
Member

Choose a reason for hiding this comment

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

ServicePointManager.ServerCertificateValidationCallback doesn't work for the Google library or .NET or both? I'm not too surprised if it doesn't work in the Google library. .NET would be more of a surprise 😄

};

var otlpExporter = new OtlpTraceExporter(exporterOptions);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Self-signed cert generated by integration test
otel-collector.crt
otel-collector.key
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,8 @@ RUN dotnet publish "OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests.csproj" -
FROM mcr.microsoft.com/dotnet/sdk:${SDK_VERSION} AS final
WORKDIR /test
COPY --from=build /drop .
ENTRYPOINT ["dotnet", "vstest", "OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests.dll"]

RUN apt-get update && apt-get install ca-certificates

# This is for testing https when using the Google gRPC library (apps prior to .NET Core 3.0).
ENV GRPC_DEFAULT_SSL_ROOTS_FILE_PATH /usr/local/share/ca-certificates/otel-collector.crt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#!/bin/bash

# Generate self-signed certificate for the collector
openssl req -new -newkey rsa:2048 -days 365 -nodes -x509 \
-subj "/CN=otel-collector" \
-keyout /otel-collector.key -out /otel-collector.crt

# Copy the certificate and private key file to shared volume that the collector
# container and test container can access
cp /otel-collector.crt /otel-collector.key /cfg

# The integration test is run via docker-compose with the --exit-code-from
# option. The --exit-code-from option implies --abort-on-container-exit
# which means when any container exits then all containers are stopped.
# Since the container running this script would be otherwise short-lived
# we sleep here. If the test does not finish within this time then the test
# container will be stopped and have a non-zero exit code.
sleep 300
Original file line number Diff line number Diff line change
@@ -1,23 +1,33 @@
# Starts an OpenTelemetry Collector and then runs the OTLP exporter integration tests.
# This should be run from the root of the repo:
# docker-compose --file=test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/docker-compose.yml --project-directory=. up --exit-code-from=tests --build
# docker-compose --file=test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/integration-test/docker-compose.yml --project-directory=. up --exit-code-from=tests --build

version: '3.7'

services:
create-cert:
image: mcr.microsoft.com/dotnet/sdk:5.0-focal
volumes:
- ./test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/integration-test:/cfg
command: /cfg/create-cert.sh

otel-collector:
image: otel/opentelemetry-collector
volumes:
- ./test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests:/cfg
command: --config=/cfg/config.yaml
- ./test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/integration-test:/cfg
command: --config=/cfg/otel-collector-config.yaml
ports:
- "4317:4317"
depends_on:
- create-cert

tests:
build:
context: .
dockerfile: ./test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/Dockerfile
command: --TestCaseFilter:CategoryName=CollectorIntegrationTests
dockerfile: ./test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/integration-test/Dockerfile
volumes:
- ./test/OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests/integration-test:/cfg
command: /cfg/run-test.sh
environment:
- OTEL_EXPORTER_OTLP_ENDPOINT=otel-collector:4317
depends_on:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ receivers:
otlp:
protocols:
grpc:
tls_settings:
cert_file: /cfg/otel-collector.crt
key_file: /cfg/otel-collector.key

exporters:
logging:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/bin/bash

# Trust the self-signed certificated used by the collector
cp /cfg/otel-collector.crt /usr/local/share/ca-certificates/
update-ca-certificates --verbose

dotnet vstest OpenTelemetry.Exporter.OpenTelemetryProtocol.Tests.dll --TestCaseFilter:CategoryName=CollectorIntegrationTests