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 for APIcast environments #644

Merged
merged 3 commits into from
Aug 4, 2021

Conversation

miguelsorianod
Copy link
Contributor

@miguelsorianod miguelsorianod commented Jul 29, 2021

This exposes OpenTracing functionality provided by APIcast into the APIManager CR for the APIcast staging and production environments.

Design

New spec.apicast.stagingSpec.openTracing and spec.apicast.produtionSpec.openTracing sections have been added in the APIManager CR.

An example:

spec:
  apicast:
    stagingSpec:
      ...
      openTracing:
        enabled: true
        tracingLibrary: jaeger
        tracingConfigRef:
          name: mysecretname
    productionSpec:
       ...
      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

The volume tracking annotation mechanism has been implemented for the opentracing-related Volumes in APIcast DeploymentConfigs in order to reconcile addition/removal of them without altering manually mounted ones.

The annotation format for them is:

apps.3scale.net/tracing-config-volume-<md5sum-of-volume-name>: <volume-name>

The Volume name format is:

tracing-config-<tracing_library>-<tracing_config_secret_name>

@miguelsorianod miguelsorianod requested a review from eguzki July 29, 2021 14:19
@miguelsorianod miguelsorianod force-pushed the add-opentracing-support branch 5 times, most recently from bd4a016 to bd78924 Compare August 3, 2021 12:28
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.

Overall looking good. I dropped some comments

pkg/3scale/amp/component/apicast.go Outdated Show resolved Hide resolved
pkg/3scale/amp/component/apicast.go Outdated Show resolved Hide resolved
pkg/3scale/amp/component/apicast_options.go Outdated Show resolved Hide resolved
pkg/3scale/amp/component/apicast_options.go Show resolved Hide resolved
pkg/3scale/amp/operator/apicast_options_provider.go Outdated Show resolved Hide resolved
@eguzki eguzki assigned miguelsorianod and unassigned eguzki Aug 4, 2021
@miguelsorianod miguelsorianod force-pushed the add-opentracing-support branch from bd78924 to 60735fa Compare August 4, 2021 13:40
@miguelsorianod miguelsorianod requested a review from eguzki August 4, 2021 13:45
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.

Some doc about how to see opentracing result when enabled?

@miguelsorianod
Copy link
Contributor Author

Some doc about how to see opentracing result when enabled?

That depends on how they install opentracing. They should see the traces in the opentracing ui if successfully working

…artialKey

to better reflect the meaning of the constant
@miguelsorianod miguelsorianod force-pushed the add-opentracing-support branch from 60735fa to bd50bdc Compare August 4, 2021 14:32
@codeclimate
Copy link

codeclimate bot commented Aug 4, 2021

Code Climate has analyzed commit bd50bdc and detected 24 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 5
Duplication 12
Style 7

View more on Code Climate.

@miguelsorianod miguelsorianod merged commit 826a4f1 into master Aug 4, 2021
@eguzki eguzki deleted the add-opentracing-support branch August 4, 2021 20:39
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