-
Notifications
You must be signed in to change notification settings - Fork 510
NGINX OpenTelemetry Input Package #15651
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
base: main
Are you sure you want to change the base?
Conversation
|
@jsoriano @cmacknz : Could you please review this first OTEL input package Point to discuss:
|
| @@ -0,0 +1,63 @@ | |||
| # NGINX OpenTelemetry Input Package | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i wonder how much of this we should just delegate to the upstream receiver. i.e. i could see an argument for literally just something like
An Elastic Agent wrapper around the nginxreceiver. Compatible with the Nginx Content Pack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am open to discussions on this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@daniela-elastic WDYT ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that, if we could delegate to the upstream receiver this means less work on our side. Do you foresee any cons to delegating to the upstream receiver? Wouldn't want to go into one way doors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tommyers-elastic : Maybe we can write this in the Metric reference section. Other parts of the documentation should be there IMO. WDYT ?
| nginx: | ||
| endpoint: {{nginx_endpoint}} | ||
| collection_interval: {{period}} | ||
| processors: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be great if agent could provide more flexibility for resource detection processing. i.e it could be a setting at the agent policy level. system seems a sensible default, but ec2 looks out of place to me here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
additionally, do you know if agent injects any batching or memory limiter processor configs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I would expect the RDP processor to be a default one by Fleet at policy level.
additionally, do you know if agent injects any batching or memory limiter processor configs?
Nothing that I see in the agent policy that is created. @jsoriano is there any ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be great if agent could provide more flexibility for resource detection processing. i.e it could be a setting at the agent policy level. system seems a sensible default, but ec2 looks out of place to me here.
Do you think most packages are going to have this resourcedetection processor? If that's the case this should be probably managed by Fleet, and not included in all packages.
additionally, do you know if agent injects any batching or memory limiter processor configs?
Nothing that I see in the agent policy that is created. @jsoriano is there any ?
Not that I know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resourcedetection should be a default global processor, and the list of global processors needs to be editable (unlike the hard coded list in agent today). I always envisioned that global processors would be associated with exporters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resourcedetectionshould be a default global processor, and the list of global processors needs to be editable (unlike the hard coded list in agent today).
So in the case of Fleet-managed integrations they should be handled by Fleet, right?
Do they need to be included in packages then as is being done here? We risk having duplicates if this starts being managed by Fleet.
I always envisioned that global processors would be associated with exporters.
There probably needs to be both, processors associated to exporters and to policies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So in the case of Fleet-managed integrations they should be handled by Fleet, right?
Yes, the default processors would have to be defined in Fleet which we don't do yet and has some problems e.g. needing a Kibana release to change the config. Creating an output package type was one idea for how to solve this.
Per policy global processors would also make sense but this could just be syntactic sugar that inserts them into each exporter pipeline.
For now are we going to go ahead with processor as part of the agent config ?
Yes there is no other way for us to do this right now. I think you would want the detectors or even the whole processor configuration to be configurable though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't ths be the nginx logo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about generating NGINX+OTEL logo. The way we have been doing for content packs.
WDYT ?
| nginx: | ||
| endpoint: {{nginx_endpoint}} | ||
| collection_interval: {{period}} | ||
| processors: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be great if agent could provide more flexibility for resource detection processing. i.e it could be a setting at the agent policy level. system seems a sensible default, but ec2 looks out of place to me here.
Do you think most packages are going to have this resourcedetection processor? If that's the case this should be probably managed by Fleet, and not included in all packages.
additionally, do you know if agent injects any batching or memory limiter processor configs?
Nothing that I see in the agent policy that is created. @jsoriano is there any ?
Not that I know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file can be removed. OTel input packages rely on pre-built component templates.
If this is the only fields file, build will fail, till we release elastic/package-spec#994 (probably this week).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is mostly going to be the only file under fields. I don't expect any mappings to go in here.
Do we actually expect predefined mappings for OTEL Input packages.
Or we can rely on the otel template and the dynamic mappings ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually expect predefined mappings for OTEL Input packages.
Or we can rely on the otel template and the dynamic mappings ?
We can rely on the pre-installed template and the dynamic mappings it includes.
We still have the option of defining fields if needed in some case, but defining fields will be completely optional for OTel input packages. It will be actually expected for OTel input packages to don't contain any field definition.
Co-authored-by: Jaime Soriano Pastor <[email protected]>
Co-authored-by: Jaime Soriano Pastor <[email protected]>
💚 Build Succeeded
History
cc @ishleenk17 |
This is a draft version of NGINX OpenTelemetry Input Package.
Since this is the very first OTEL Input package, this PR is being used to figure out the structure, template, what works what doesn't while creating packages.
FYI, using HTTPChecker example as reference
Relates: https://github.com/elastic/obs-integration-team/issues/622