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

Expose Jaeger agent port as environment variable on deployment manifest #396

Closed
masroorhasan opened this issue Jan 18, 2019 · 1 comment
Closed

Comments

@masroorhasan
Copy link
Contributor

masroorhasan commented Jan 18, 2019

The default configuration for Jaeger tracing allows specifying jaeger agent host address but hardcodes port 5775 (for zipkin.thrift over thrift protocol).

jaeger_serv = os.environ.get("JAEGER_AGENT_HOST","0.0.0.0")
        jaeger_config = os.environ.get("JAEGER_CONFIG_PATH",None)
        if jaeger_config is None:
            logger.info("Using default tracing config")
            config = Config(
                config={ # usually read from some yaml config
                    'sampler': {
                        'type': 'const',
                        'param': 1,
                    },
                    'local_agent': {
                        'reporting_host': jaeger_serv,
                        'reporting_port': 5775,
                    },
                    'logging': True,
                },
                service_name=args.interface_name,
                validate=True,
            )

However, Jaeger agent exposes multiple UDP ports to support jaeger.thrift and binary thrift as well: https://www.jaegertracing.io/docs/1.8/deployment/#agent

The default seldon python deployment should expose enabling the agent port through environment variable so users can pick the right port to configure clients (for example, 6831).

Setting that with configuration YAML mounted on config map does not cover all use cases of setting host and port. For example, Jaeger agent works best deployed as a Daemonset on each Node, with deployments scheduled on Node only needs to send data to its local agent. Setting status.hostIP on the data in a config map is not supported native (could mount and sed update the value but that seems like a hack).

Basically, proposing to support this on a seldon deployment:

         - name: JAEGER_AGENT_HOST
            valueFrom:
              fieldRef:
                fieldPath: status.hostIP
          - name: JAEGER_AGENT_PORT
             value: "6831"

Proposed change: master...masroorhasan:masroor/jaeger-port

If the above change is ok, I'd like to submit a PR (pending discussion on the change). I assume docs and example would need to be updated as well.

Thanks!

@ukclivecox
Copy link
Contributor

Your proposed change makes sense.
We should fully support Jaeger configuration.
Please do a PR.

agrski added a commit that referenced this issue Dec 2, 2022
* Use Distroless Debian11 for Hodometer image

* Use Distroless Debian11 for Hodometer stub receiver image

* Use Distroless static for Hodometer images

* Use nonroot Distroless images for Hodometer

* Use nonroot Distroless images for SCv2 core Go components

* Use Debian-based Distroless image for core Go components

These components are buit with Kafka, which uses librdkafka under the hood.
In turn, this is highly likely to be dynamically linked to a C/C++ runtime,
meaning we need an image which contains this C/C++ dependency.

* Disable CGo for scheduler container build

This successfully allows the scheduler container to start in Compose.

* Change scheduler base image back to static Distroless

* Disable CGo for agent images & use static Distroless image as base

* Disable CGo for scheduler, agent, and proxy scheduler binaries in Makefile

* Add Dockerfile comments re use of specific base images for built binaries

* Use entrypoint instead of cmd in Dockerfiles
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

No branches or pull requests

2 participants