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

python/iot3: make OTLP optional in simple API #259

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ymorin-orange
Copy link
Member

@ymorin-orange ymorin-orange commented Jan 14, 2025

Changes:

  • Feature: allow using the simple API without OTLP

Closes: #236


How to test:

  1. Pre-requisites:
    • a python 3.11 environment; if you do not have one already, use a container:
      $ docker container run \
          --detach \
          --name iot3 \
          --rm \
          -ti \
          --network host \
          -e http_proxy \
          -e https_proxy \
          -e no_proxy \
          --user $(id -u):$(id -u) \
          --mount type=bind,source=$(pwd),destination=$(pwd) \
          --workdir $(pwd) \
          python:3.11.9-slim-bookworm \
          /bin/bash -il
      $ docker container exec -u 0:0 iot3 apt update
      $ docker container exec -u 0:0 iot3 apt install -y git
      $ docker container attach iot3
      Note: in the following, the $ prompt means running in a shell that has access to python3.11, while the 🐍 $ prompt means running in the activated venv.
    • an OpenTelemetry collector on localhost:
      $ docker container run \
          --rm \
          -p 16686:16686 \
          -p 4318:4318 \
          jaegertracing/all-in-one:1.58
  2. Install the IoT3 SDK package:
    $ python3.11 -m venv /tmp/iot3
    $ . /tmp/iot3/bin/activate
    🐍 $ pip install python/iot3
  3. Run the IoT3 test-suite for the simple API:
    🐍 $ python/iot3/tests/test-iot3-core
    

Expected results:

  1. The IoT3 SDK package is isntalled
  2. The test-suite succeeds:
    • the test reports ([...] is a hex string):
      IoT3 Core SDK with MQTT and OTLP
      test/[...]/iot3/passed: b'passed'
      IoT3 Core SDK with MQTT and no OTLP
      test/[...]/iot3/no-otlp/passed: b'passed'
      
    • The Jaeger UI (on http://localhost:16686/) contains exactly three traces which MQTT topic (in Tags -> iot3.core.mqtt.topic) does not contain the no-otlp component:
      • one trace reports a failure due to not being connected;
      • one trace reports a publish;
      • one trace reports a receive with a reference to the publish, above.

... and fix my own identity while at it.

Fixes: Orange-OpenSource#258

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
Currently, we can create an MQTT client either by passing an explicit
OTLP span manager, whether real or fake, or by not passing anything, in
which case no OTLP traces would be sent.

However, for some callers, it might be simpler to explicitly pass 'None'
to not send OTLP traces, rahter than pass a fake span manager.

Change the MQTT signature to accept this new use-case, in a backward
compatible way.

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
Currently, using OTLP is mandatory in the simple API. However, it is
perfectly legit for a user to only be interested in sending MQTT
messages.

Similarly, it is possible that a bootstrap reply does not contain any
OTLP setup when the IoT3 instance does not offer an OTLP collector

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
We currently use a loop to iterate over all the possible compression and
authentication schemes, and compare their string representation against
the valude from the configuration, which is a string.

However, python's enum.strEnum() constructor does accept a string as a
parameter, where that string is the str() representation of one of the
enum values, which incidentally also plays the role of validating the
value from the configuration.

Use that, rather than our canned, ugly for-loop.

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
@ymorin-orange ymorin-orange force-pushed the nyma/gh-236.optional-otlp branch from 505e063 to 52aa2d9 Compare January 14, 2025 12:35
@ymorin-orange
Copy link
Member Author

The rust workflow fails for reasons unrelated to this PR, as no rust code was touched here.

@ymorin-orange ymorin-orange marked this pull request as ready for review January 14, 2025 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make telemetry optionnal
1 participant