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

Add OpenTracing configurability #141

Merged
merged 1 commit into from
Aug 6, 2021
Merged

Conversation

miguelsorianod
Copy link
Contributor

@miguelsorianod miguelsorianod commented Jul 21, 2021

This exposes OpenTracing functionality provided by APIcast into the APIcast operator.

Design

A new spec.openTracing section has been added in the APIcast CR.

An example:

spec:
  openTracing:
    enabled: true
    tracingLibrary: jaeger
    tracingConfigRef:
      name: mysecretname
  • All attributes in the openTracing section are optional
  • If the enabled flag is not specified, it defaults to false
  • If the tracingLibrary flag is not specified, it defaults to jaeger
  • If the tracingConfigRef flag is not specified, it defaults to a default tracing configuration specific to the set tracingLirary. This default tracing configuration is provided in the APIcast image itself

The configuration is mounted into the APIcast pod by looking at a config key in the provided secret name and mounted in /opt/app-root/src/tracing-configs/tracing-config-<tracing_library_name>-<secret_name>, for example in opt/app-root/src/tracing-configs/tracing-config-jaeger-mysecretname

Pending:

  • Verification

@miguelsorianod
Copy link
Contributor Author

With the current implementation the OpenTracing configuration is being provided with a Secret but in the official APIcast product documentation I see a ConfigMap is used. Maybe in this case a Secret is not needed and we should use a ConfigMap instead?

@miguelsorianod miguelsorianod changed the title Add OpenTracing configurability [WIP] Add OpenTracing configurability Jul 21, 2021
@miguelsorianod
Copy link
Contributor Author

Ready for a first review @eguzki before I proceed to do some verifications

@miguelsorianod miguelsorianod force-pushed the add-opentracing-support branch 2 times, most recently from 45e0ac7 to 8fd2ee7 Compare July 21, 2021 14:50
pkg/apicast/apicast.go Outdated Show resolved Hide resolved
Copy link
Member

@eguzki eguzki left a comment

Choose a reason for hiding this comment

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

looks good

@miguelsorianod miguelsorianod force-pushed the add-opentracing-support branch 2 times, most recently from f9c721b to cdc8e8e Compare July 22, 2021 11:23
@miguelsorianod miguelsorianod changed the title [WIP] Add OpenTracing configurability Add OpenTracing configurability Jul 22, 2021
@miguelsorianod
Copy link
Contributor Author

@eguzki I think this should be ready

Copy link
Member

@eguzki eguzki left a comment

Choose a reason for hiding this comment

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

LGTM

minor comments dropped

apis/apps/v1alpha1/apicast_types.go Outdated Show resolved Hide resolved
pkg/apicast/apicast_option_provider.go Outdated Show resolved Hide resolved
@miguelsorianod miguelsorianod merged commit f3b486c into master Aug 6, 2021
@miguelsorianod miguelsorianod deleted the add-opentracing-support branch August 6, 2021 14:11
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.

2 participants