search

openclarity / apiclarity

An API security tool to capture and analyze API traffic, test API endpoints, reconstruct Open API specification, and identify API security risks. 
https://apiclarity.io
Apache License 2.0
504 stars 64 forks source link

Otel collector plugin #255

Closed jnapper7 closed 1 year ago

jnapper7 commented 1 year ago

This PR adds a plugin for the OpenTelemetry Collector. Specifically, it adds an exporter for the collector that exports OTel traces that are server/client to APIClarity in the Telemetry format. The plugin dir includes build of a docker image of the otel collector with the addition of the APIClarity exporter plugin.

jeff-trianglecm commented 1 year ago

  1. How will release happen? what is being built and what the user need to build? What the Dockerfile is for?

    The Dockerfile builds the OpenTelemetry collector with the repo's APIClarity exporter included. The OTel collector plugins are golang packages. The Dockerfile pulls a recent OTel collector config and adds in the APIClarity exporter, rebuilding everything to additionally include the exporter (and everything else already included in the default OTel collector).

    Eventually the apiclarityexporter package should be available on (the public Go package list)[https://pkg.go.dev/] because I would like to contribute the exporter to the main OTel collector repo. When it is added there, the APIClarity exporter would be generally available and you would not need to build your own version; you could simply configure the default version.

    The container and the apiclarityexporter package are the only build artefacts.

  2. Should this be part of Helm installation/configuration?

    No, I don't believe so. The plugin would only be configured in the OTel collector's configuration. An example of that config is here: https://github.com/openclarity/apiclarity/blob/otel-collector-plugin/plugins/otel-collector/configs/otel-collector-config.yaml.

  3. Should trace sampling manager be integrated into the otel exporter?

    Yes, maybe? Sampling is already handled by the tracing infrastructure, both at the SDK level where tracing is generated and within the OTel collector itself by selecting different traces. It might be confusing to the user if the exporter also subsampled, but the config for that sampling was outside of the OTel configuration. Then again, maybe not?

  4. what are the "replaces" commits in the readme?

    Does this refer to the instructions on building the OTel collector at the bottom? These are used to build the OTel collector using the APIClarity exporter plugin package. Since there is no public version of the package yet, it has to be built locally using the local package. The replacement for the APIClarity api package is due to the crazy build packages of APIClarity itself.

  5. Should be added to the main readme of APIClarity?

    I'm happy to add it as one of the example source integrations, which is where it would be relevant.

FrimIdan commented 1 year ago
  1. The container and the apiclarityexporter package are the only build artefacts.

So, do you want/need to the container image during the release flow as we are doing to all other containers?

Eventually the apiclarityexporter package should be available on (the public Go package list)[https://pkg.go.dev/]

How it will become public? Once it will be pushed to master and tagged we can remove all the replaces in the readme?

jnapper7 commented 1 year ago
  1. The container and the apiclarityexporter package are the only build artefacts.

So, do you want/need to the container image during the release flow as we are doing to all other containers?

I think releasing the docker container makes sense along with the normal release flow so it should be added to the release action. Do you want me to do that?

Eventually the apiclarityexporter package should be available on (the public Go package list)[https://pkg.go.dev/]

How it will become public? Once it will be pushed to master and tagged we can remove all the replaces in the readme?

Yes, I think so.

FrimIdan commented 1 year ago
  1. The container and the apiclarityexporter package are the only build artefacts.

So, do you want/need to the container image during the release flow as we are doing to all other containers?

I think releasing the docker container makes sense along with the normal release flow so it should be added to the release action. Do you want me to do that?

Yes please, it should be added to docker.yaml and release.yaml workflow

Eventually the apiclarityexporter package should be available on (the public Go package list)[https://pkg.go.dev/]

How it will become public? Once it will be pushed to master and tagged we can remove all the replaces in the readme?

Yes, I think so.

OK, understand, so once it will be pushed and tagged we can check if that works and update the docs to remove the replace stuff.

jnapper7 commented 1 year ago
  1. The container and the apiclarityexporter package are the only build artefacts.

So, do you want/need to the container image during the release flow as we are doing to all other containers?

I think releasing the docker container makes sense along with the normal release flow so it should be added to the release action. Do you want me to do that?

Yes please, it should be added to docker.yaml and release.yaml workflow

The last commit finally does this; however, I cannot really test it myself. :(

Eventually the apiclarityexporter package should be available on (the public Go package list)[https://pkg.go.dev/]

How it will become public? Once it will be pushed to master and tagged we can remove all the replaces in the readme?

Yes, I think so.

OK, understand, so once it will be pushed and tagged we can check if that works and update the docs to remove the replace stuff.

FrimIdan commented 1 year ago

The last commit finally does this; however, I cannot really test it myself. :(

Best way to check it is to fork the project to your local account, then you can test any action with any condition (you can run the build action on any push to your side-branch for example)

jnapper7 commented 1 year ago

The last commit finally does this; however, I cannot really test it myself. :(

Best way to check it is to fork the project to your local account, then you can test any action with any condition (you can run the build action on any push to your side-branch for example)

Ok, I've checked the docker workflow, but the release workflow is stuck for the same reason as the above CI/build problem. :(

FrimIdan commented 1 year ago

The last commit finally does this; however, I cannot really test it myself. :(

Best way to check it is to fork the project to your local account, then you can test any action with any condition (you can run the build action on any push to your side-branch for example)

Ok, I've checked the docker workflow, but the release workflow is stuck for the same reason as the above CI/build problem. :(

Saw the problem, not sure why it is running lint on non relevant code, did you tried to run it locally? Can you maybe try to remove the use of make -C ... and do

cd plugins/otel-collector/apiclarityexporter && ../../../bin/golangci-lint run
jnapper7 commented 1 year ago

The last commit finally does this; however, I cannot really test it myself. :(

Best way to check it is to fork the project to your local account, then you can test any action with any condition (you can run the build action on any push to your side-branch for example)

Ok, I've checked the docker workflow, but the release workflow is stuck for the same reason as the above CI/build problem. :(

Saw the problem, not sure why it is running lint on non relevant code, did you tried to run it locally? Can you maybe try to remove the use of make -C ... and do

cd plugins/otel-collector/apiclarityexporter && ../../../bin/golangci-lint run

Indeed. I have tried so many things. :)

At the moment it passes b/c I specifically include the broken package that doesn't pass typecheck. I did align the Makefiles so they should all be the same now.

Let me know if ignoring the one package for now is fine or not. I really have tried everything that I could think of (local run, changing the package version, etc.).

jnapper7 commented 1 year ago

The last commit finally does this; however, I cannot really test it myself. :(

Best way to check it is to fork the project to your local account, then you can test any action with any condition (you can run the build action on any push to your side-branch for example)

I have verified this in a fork in my own repo here: Forked Actions Note the Release action only fails to publish Helm, which I specifically didn't authorize b/c I naturally didn't want to publish it. :)

FrimIdan commented 1 year ago

Let me know if ignoring the one package for now is fine or not

It's fine by me

FrimIdan commented 1 year ago

5. I'm happy to add it as one of the example source integrations, which is where it would be relevant.

@jnapper7 We are missing that one and we probably can ship it