make options passing by value

Author: lychung83

monitoring/exporter/stackdriver/mock_check_test.go

@@ -127,7 +127,7 @@ func mockAddToBundler(bndler *bundler.Bundler, item interface{}, _ int) error {
 
 // newTestExp creates an exporter which saves error to errStorage. Caller should not set
 // opts.OnError.
-func newTestExp(t *testing.T, opts *Options) *Exporter {
+func newTestExp(t *testing.T, opts Options) *Exporter {
 	opts.OnError = testOnError
 	exp, err := NewExporter(ctx, opts)
 	if err != nil {
@@ -140,7 +140,7 @@ func newTestExp(t *testing.T, opts *Options) *Exporter {
 
 // newTestProjData creates a projectData object to test behavior of projectData.uploadRowData. Other
 // uses are not recommended. As newTestExp, all errors are saved to errStorage.
-func newTestProjData(t *testing.T, opts *Options) *projectData {
+func newTestProjData(t *testing.T, opts Options) *projectData {
 	return newTestExp(t, opts).newProjectData(project1)
 }
 

monitoring/exporter/stackdriver/stackdriver.go

@@ -47,12 +47,7 @@ type Exporter struct {
 	// TODO(lawrencechung): If possible, find a way to not storing ctx in the struct.
 	ctx    context.Context
 	client *monitoring.MetricClient
-	opts   *Options
-
-	// copy of some option values which may be modified by exporter.
-	getProjectID func(*RowData) (string, error)
-	onError      func(error, ...*RowData)
-	makeResource func(*RowData) (*mrpb.MonitoredResource, error)
+	opts   Options
 
 	// mu protects access to projDataMap
 	mu sync.Mutex
@@ -146,7 +141,7 @@ var (
 // NewExporter creates an Exporter object. Once a call to NewExporter is made, any fields in opts
 // must not be modified at all. ctx will also be used throughout entire exporter operation when
 // making RPC call.
-func NewExporter(ctx context.Context, opts *Options) (*Exporter, error) {
+func NewExporter(ctx context.Context, opts Options) (*Exporter, error) {
 	client, err := newMetricClient(ctx, opts.ClientOptions...)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create a metric client: %v", err)
@@ -159,19 +154,14 @@ func NewExporter(ctx context.Context, opts *Options) (*Exporter, error) {
 		projDataMap: make(map[string]*projectData),
 	}
 
-	// We don't want to modify user-supplied options, so save default options directly in
-	// exporter.
-	e.getProjectID = defaultGetProjectID
-	if opts.GetProjectID != nil {
-		e.getProjectID = opts.GetProjectID
+	if e.opts.GetProjectID == nil {
+		e.opts.GetProjectID = defaultGetProjectID
 	}
-	e.onError = defaultOnError
-	if opts.OnError != nil {
-		e.onError = opts.OnError
+	if e.opts.OnError == nil {
+		e.opts.OnError = defaultOnError
 	}
-	e.makeResource = defaultMakeResource
-	if opts.MakeResource != nil {
-		e.makeResource = opts.MakeResource
+	if e.opts.MakeResource == nil {
+		e.opts.MakeResource = defaultMakeResource
 	}
 
 	return e, nil
@@ -206,12 +196,12 @@ var RowDataNotApplicableError = errors.New("row data is not applicable to the ex
 
 // exportRowData exports a single row data.
 func (e *Exporter) exportRowData(rd *RowData) {
-	projID, err := e.getProjectID(rd)
+	projID, err := e.opts.GetProjectID(rd)
 	if err != nil {
 		// We ignore non-applicable RowData.
 		if err != RowDataNotApplicableError {
 			newErr := fmt.Errorf("failed to get project ID on row data with view %s: %v", rd.View.Name, err)
-			e.onError(newErr, rd)
+			e.opts.OnError(newErr, rd)
 		}
 		return
 	}
@@ -222,7 +212,7 @@ func (e *Exporter) exportRowData(rd *RowData) {
 		go pd.uploadRowData(rd)
 	default:
 		newErr := fmt.Errorf("failed to add row data with view %s to bundle for project %s: %v", rd.View.Name, projID, err)
-		e.onError(newErr, rd)
+		e.opts.OnError(newErr, rd)
 	}
 }
 

monitoring/exporter/stackdriver/stackdriver_test.go

@@ -76,7 +76,7 @@ func testProjectClassifyNoError(t *testing.T) {
 		}
 	}
 
-	exp := newTestExp(t, &Options{GetProjectID: getProjectID})
+	exp := newTestExp(t, Options{GetProjectID: getProjectID})
 	exp.ExportView(viewData1)
 	exp.ExportView(viewData2)
 
@@ -122,7 +122,7 @@ func testProjectClassifyError(t *testing.T) {
 		}
 	}
 
-	exp := newTestExp(t, &Options{GetProjectID: getProjectID})
+	exp := newTestExp(t, Options{GetProjectID: getProjectID})
 	exp.ExportView(viewData1)
 	exp.ExportView(viewData2)
 
@@ -171,7 +171,7 @@ func testDefaultProjectClassify(t *testing.T) {
 		Rows:  []*view.Row{view3row3},
 	}
 
-	exp := newTestExp(t, &Options{})
+	exp := newTestExp(t, Options{})
 	exp.ExportView(viewData1)
 	exp.ExportView(viewData2)
 	exp.ExportView(viewData3)
@@ -194,7 +194,7 @@ func testDefaultProjectClassify(t *testing.T) {
 // testUploadNoError tests that all RowData objects passed to uploadRowData() are grouped by
 // slice of length MaxTimeSeriesPerUpload, and passed to createTimeSeries().
 func testUploadNoError(t *testing.T) {
-	pd := newTestProjData(t, &Options{})
+	pd := newTestProjData(t, Options{})
 	rd := []*RowData{
 		{view1, startTime1, endTime1, view1row1},
 		{view1, startTime1, endTime1, view1row2},
@@ -220,7 +220,7 @@ func testUploadTimeSeriesMakeError(t *testing.T) {
 		}
 		return defaultMakeResource(rd)
 	}
-	pd := newTestProjData(t, &Options{MakeResource: makeResource})
+	pd := newTestProjData(t, Options{MakeResource: makeResource})
 	rd := []*RowData{
 		{view1, startTime1, endTime1, view1row1},
 		{view1, startTime1, endTime1, view1row2},
@@ -255,7 +255,7 @@ func testUploadTimeSeriesMakeError(t *testing.T) {
 // testUploadTimeSeriesMakeError tests that exporter can handle error on metric client's time
 // series create RPC call.
 func testUploadWithMetricClientError(t *testing.T) {
-	pd := newTestProjData(t, &Options{})
+	pd := newTestProjData(t, Options{})
 	timeSeriesResults = append(timeSeriesResults, invalidDataError)
 	rd := []*RowData{
 		{view1, startTime1, endTime1, view1row1},
@@ -298,7 +298,7 @@ func testMakeResource(t *testing.T) {
 			return nil, unrecognizedDataError
 		}
 	}
-	pd := newTestProjData(t, &Options{MakeResource: makeResource})
+	pd := newTestProjData(t, Options{MakeResource: makeResource})
 	rd := []*RowData{
 		{view1, startTime1, endTime1, view1row1},
 		{view1, startTime1, endTime1, view1row2},
@@ -318,7 +318,7 @@ func testMakeResource(t *testing.T) {
 // testMakeLabel tests that exporter can correctly handle label manipulation process, including
 // merging default label with tags, and removing unexported labels.
 func testMakeLabel(t *testing.T) {
-	opts := &Options{
+	opts := Options{
 		DefaultLabels: map[string]string{
 			label1name: value7,
 			label4name: value8,

monitoring/exporter/stackdriver/upload.go

@@ -44,7 +44,7 @@ func (pd *projectData) uploadRowData(bundle interface{}) {
 	for _, rd := range rds {
 		ts, err := exp.makeTS(rd)
 		if err != nil {
-			exp.onError(err, rd)
+			exp.opts.OnError(err, rd)
 			continue
 		}
 		// Time series created. We update both uploadTs and uploadRds.
@@ -68,7 +68,7 @@ func (e *Exporter) makeTS(rd *RowData) (*mpb.TimeSeries, error) {
 	if pt.Value == nil {
 		return nil, fmt.Errorf("inconsistent data found in view %s", rd.View.Name)
 	}
-	resource, err := e.makeResource(rd)
+	resource, err := e.opts.MakeResource(rd)
 	if err != nil {
 		return nil, fmt.Errorf("failed to construct resource of view %s: %v", rd.View.Name, err)
 	}
@@ -102,7 +102,7 @@ func (e *Exporter) makeLabels(tags []tag.Tag) map[string]string {
 }
 
 // uploadTimeSeries uploads timeSeries. ts and rds must contain matching data, and ts must not be
-// empty. When uploading fails, this function calls exporter's onError() directly, not propagating
+// empty. When uploading fails, this function calls exporter's OnError() directly, not propagating
 // errors to the caller.
 func (pd *projectData) uploadTimeSeries(ts []*mpb.TimeSeries, rds []*RowData) {
 	exp := pd.parent
@@ -113,6 +113,6 @@ func (pd *projectData) uploadTimeSeries(ts []*mpb.TimeSeries, rds []*RowData) {
 	if err := createTimeSeries(exp.client, exp.ctx, req); err != nil {
 		newErr := fmt.Errorf("RPC call to create time series failed for project %s: %v", pd.projectID, err)
 		// We pass all row data not successfully uploaded.
-		exp.onError(newErr, rds...)
+		exp.opts.OnError(newErr, rds...)
 	}
 }