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 code and scheme generation #20

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/subtests/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
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Feb 23, 2024
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.

🚀

cmd/generator/main.go Outdated Show resolved Hide resolved
cmd/generator/code_test.go Outdated Show resolved Hide resolved
cmd/generator/main.go Outdated Show resolved Hide resolved
cmd/generator/scheme.go Show resolved Hide resolved
cmd/generator/scheme_test.go Outdated Show resolved Hide resolved
cmd/generator/tests/data.go Outdated Show resolved Hide resolved
cmd/generator/tests/subtests/data2.go Outdated Show resolved Hide resolved
cmd/generator/parser.go Outdated Show resolved Hide resolved
cmd/generator/parser.go Outdated Show resolved Hide resolved
@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
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
Copy link
Contributor Author

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

@pleshakov pleshakov mentioned this pull request Feb 28, 2024
6 tasks
Copy link
Member

@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
8 checks passed
@pleshakov pleshakov deleted the feature/code-and-scheme-gen branch February 29, 2024 15:16
@pleshakov pleshakov mentioned this pull request Feb 29, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement Pull requests for new features/feature enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants