8000 Add code and scheme generation by pleshakov · Pull Request #20 · nginx/telemetry-exporter · GitHub
[go: up one dir, main page]
Skip to content

Add code and scheme generation #20

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Feb 29, 2024
Merged

Add code and scheme generation #20

merged 10 commits into from
Feb 29, 2024

Conversation

pleshakov
Copy link
Contributor

Proposed changes

Problem:

  • Ensure the Attributes method of Exprotable interface are generated automatically for the telemetry data types.
  • Ensure the avro scheme (.avdl) is generated automatically for the telemetry data types.

Solution:

  • Add generator tool in cmd/generator that generates code (Attributes method) and the scheme.
  • Generator can be used in //go:generate annotations in go source files.
  • Generator has "generator" build tag so that it is not included into the telemetry library by default.
  • Generator uses golang.org/x/tools/go/packages to parse source files.

Testing:

  • Unit tests of parsing.
  • Unit tests of code and scheme generation - ensuring non-zero output.
  • Unit tests of generated code in cmd/generator/tests package
  • Manual validation of the generated scheme (cmd/generator/tests/data.avdl) using Apache Avro IDL Scheme Support IntelliJ plugin.

CLOSES - #18

Note: if you'd like to experiment with code and scheme generation:

  • update Data in cmd/generator/tests/data.go and/or Data2 in cmd/generator/tests/su 8000 btests/data2.go
  • run make generate
  • check the output of cmd/generator/tests/data_attributes_generated.go, cmd/generator/tests/data.avdl and cmd/generator/tests/subtests/data2_attributes_generated.go

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING
    guide
  • I have proven my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have ensured the README is up to date
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch on my own fork

@pleshakov pleshakov requested a review from a team as a code owner February 23, 2024 22:59
@github-actions github-actions bot added the enhancement Pull requests for new features/feature enhancements label Feb 23, 2024
@pleshakov pleshakov force-pushed the feature/code-and-scheme-gen branch from ab543a9 to 8951616 Compare February 23, 2024 23:01
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Feb 23, 2024
jjngx
jjngx approved these changes Feb 26, 2024
View reviewed changes
Copy link
Contributor
@jjngx jjngx left a comment

Choose a reason for hiding this comment

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

🚀

ciarams87
ciarams87 reviewed Feb 28, 2024
View reviewed changes
ciarams87
ciarams87 reviewed Feb 28, 2024
View reviewed changes
sjberman
sjberman reviewed Feb 28, 2024
View reviewed changes
@pleshakov
Copy link
Contributor Author

during meeting with @shaun-nx , we found two small bugs:

(1)
if generator generates attributes for a type declared in telemetry package github.com/nginxinc/telemetry-exporter/pkg/telemetry, the generated code will fail because it of the self-import

package telemetry
/*
This is a generated file. DO NOT EDIT.
*/

import (
	"go.opentelemetry.io/otel/attribute"

	"github.com/nginxinc/telemetry-exporter/pkg/telemetry" // self-import
)

(2)
When generated code in project like NGF in package telemetry (like github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/telemetry), the generated code import this repo package telemetry "github.com/nginxinc/telemetry-exporter/pkg/telemetry, which creates problems. Solution - add alias to the exported telemetry package

package telemetry
/*
This is a generated file. DO NOT EDIT.
*/

import (
	"go.opentelemetry.io/otel/attribute"
	"github.com/nginxinc/telemetry-exporter/pkg/telemetry"
)

func (d *NGFResourceCounts2) Attributes() []attribute.KeyValue {
	var attrs []attribute.KeyValue

	attrs = append(attrs, attribute.Int64("Gateways", d.Gateways))
	

	return attrs
}

// problem - we want to use github.com/nginxinc/telemetry-exporter/pkg/telemetry.Exportable here, but end up using Exportable from this package.
var _ telemetry.Exportable = (*NGFResourceCounts2)(nil)

I will fix them shortly

pleshakov and others added 10 commits February 28, 2024 12:51
@pleshakov @sjberman @ciarams87
Add code and scheme generation
530a4fb 
Problem:
- Ensure the Attributes method of Exprotable interface are generated
automatically for the telemetry data types.
- Ensure the avro scheme (.avdl) is generated automatically for the
telemetry data types.

Solution:
- Add generator tool in cmd/generator that generates code (Attributes
method) and the scheme.
- Generator can be used in //go:generate annotations in go source files.
- Generator has "generator" build tag so that it is not included into
the telemetry library by default.
- Generator uses golang.org/x/tools/go/packages to parse source files.

Testing:
- Unit tests of parsing.
- Unit tests of code and scheme generation - ensuring non-zero output.
- Unit tests of generated code in cmd/generator/tests package
- Manual validation of the generated scheme
  (cmd/generator/tests/data.avdl) using Apache Avro IDL Scheme Support
  IntelliJ plugin.

CLOSES - #18

Co-authored-by: Saylor Berman <s.berman@f5.com>
Co-authored-by: Ciara Stacke <c.stacke@f5.com>
@pleshakov
existWithUsage -> exitWithUsage
9f2926e
@pleshakov
-> Expect(...).To(Succeed())
78c2dbf
@pleshakov
Remove "and" in the flag usage message
020a0cc
@pleshakov
Add clarifying comment about disabling govet
32abe88
@pleshakov
Shorten function signatures
5094646
@pleshakov
Fix comment
9a4d13d
@pleshakov
Data2 -> AnotherData
f676004
@pleshakov
Fix a bug when parsed struct in a package named telemetry
05d1255
@pleshakov
Fix a bug when parsing a struct in the github.com/nginxinc/telemetry-…
86bf656 
…exporter/pkg/telemetry package
@pleshakov pleshakov force-pushed the feature/code-and-scheme-gen branch from 8951616 to 86bf656 Compare February 28, 2024 21:02
@pleshakov
Copy link
Contributor Author

(forced push to resolve go.mod conflicts with the main branch)

@pleshakov pleshakov mentioned this pull request Feb 28, 2024
Merged
6 tasks
ciarams87
ciarams87 approved these changes Feb 29, 2024
View reviewed changes
Copy 8000 link
Contributor
@ciarams87 ciarams87 left a comment

Choose a reason for hiding this comment

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

LGTM

@pleshakov pleshakov merged commit 3fc010d into main Feb 29, 2024
@pleshakov pleshakov deleted the feature/code-and-scheme-gen branch February 29, 2024 15:16
@pleshakov pleshakov mentioned this pull request Feb 29, 2024
Merged
6 tasks
@pleshakov pleshakov mentioned this pull request Feb 29, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjberman sjberman sjberman approved these changes

@ciarams87 ciarams87 ciarams87 approved these changes

@jjngx jjngx jjngx approved these changes

@shaun-nx shaun-nx shaun-nx approved these changes

Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file enhancement Pull requests for new features/feature enhancements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pleshakov @sjberman @ciarams87 @jjngx @shaun-nx
0